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

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

[Dan Carpenter]

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;

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.

    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

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

[Masami Hiramatsu]

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 <error27@gmail.com> 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) <mhiramat@kernel.org>

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

[Dan Carpenter]

On Tue, May 12, 2026 at 09:16:38AM +0900, Masami Hiramatsu wrote:
> 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 <error27@gmail.com> 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...)
> 

Sashiko says that pointer math on a NULL is undefined but we do it all
the time in the kernel...  When you are a the 800 pound gorilla, you can
ask compilers to implement features the way you want them to be.  :P

regards,
dan carpenter

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

[Breno Leitao]

On Tue, May 12, 2026 at 09:16:38AM +0900, Masami Hiramatsu wrote:
> 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 <error27@gmail.com> 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()

For your information, I am moving this function to lib/bootconfig.

https://lore.kernel.org/all/20260508-bootconfig_using_tools-v1-1-1132219aa773@debian.org/

I understand that no action is required on this report, correct?

Thanks,
--breno