Re: [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line

Masami Hiramatsu (Google) <[email protected]>
Newsgroups gmane.linux.kernel.janitors,gmane.linux.kernel
Message-ID <[email protected]>
Hi Dan,

Thanks for reporting. A similar problem is pointed by Sashiko [1].

[1] https://sashiko.dev/#/patchset/20260508-bootconfig_using_tools-v1-0-1132219aa773%40debian.org

On Fri, 8 May 2026 20:07:25 +0300
Dan Carpenter <[email protected]> wrote:

> Hello Masami Hiramatsu,
> 
> Commit 51887d03aca1 ("bootconfig: init: Allow admin to use bootconfig
> for kernel command line") from Jan 11, 2020 (linux-next), leads to
> the following Smatch static checker warning:
> 
> 	init/main.c:368 xbc_snprint_cmdline()
> 	use scnprintf() instead of snprintf()
> 
> init/main.c
>     331 static int __init xbc_snprint_cmdline(char *buf, size_t size,
>     332                                       struct xbc_node *root)
>     333 {
>     334         struct xbc_node *knode, *vnode;
>     335         char *end = buf + size;
>     336         const char *val, *q;
>     337         int ret;
>     338 
>     339         xbc_node_for_each_key_value(root, knode, val) {
>     340                 ret = xbc_node_compose_key_after(root, knode,
>     341                                         xbc_namebuf, XBC_KEYLEN_MAX);
>     342                 if (ret < 0)
>     343                         return ret;
>     344 
>     345                 vnode = xbc_node_get_child(knode);
>     346                 if (!vnode) {
>     347                         ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
>     348                         if (ret < 0)
>     349                                 return ret;
>     350                         buf += ret;
> 
> In user space snprintf() can return negative, but in the kernel, no.
> It returns the number of bytes (not counting the NUL terminator) which
> would have been copied if there were enough space.  So maybe you want
> to do something like:
> 
> 	remain = rest(buf, end);
> 	ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
> 	if (ret >= remain)
> 		return -ENOSPC;

Actually, we need to query the length of required buffer size if buf == NULL
or the buffer size is not enough.

But as Sashiko pointed, I need to check it with UBSAN. (but I think,
even if @buf is NULL, the @buf is char *, thus it is safe to add some
value...)

> 
> Or maybe you might want to use scnprintf() which returns the number of
> bytes actually copied.  Otherwise bug ends up pointing to beyond the end
> of the buffer.

No, I need to calculate the required length of buffer.

Thank you,

> 
>     351                         continue;
>     352                 }
>     353                 xbc_array_for_each_value(vnode, val) {
>     354                         /*
>     355                          * For prettier and more readable /proc/cmdline, only
>     356                          * quote the value when necessary, i.e. when it contains
>     357                          * whitespace.
>     358                          */
>     359                         q = strpbrk(val, " \t\r\n") ? "\"" : "";
>     360                         ret = snprintf(buf, rest(buf, end), "%s=%s%s%s ",
>                                 ^^^^^^^^^^^^^^^
> Same.
> 
>     361                                        xbc_namebuf, q, val, q);
>     362                         if (ret < 0)
>     363                                 return ret;
>     364                         buf += ret;
>     365                 }
>     366         }
>     367 
> --> 368         return buf - (end - size);
>     369 }
> 
> This email is a free service from the Smatch-CI project [smatch.sf.net].
> 
> regards,
> dan carpenter


-- 
Masami Hiramatsu (Google) <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.