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

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