* [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line
@ 2026-05-08 17:07 Dan Carpenter
2026-05-12 0:16 ` Masami Hiramatsu
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2026-05-08 17:07 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: kernel-janitors
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line
2026-05-08 17:07 [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line Dan Carpenter
@ 2026-05-12 0:16 ` Masami Hiramatsu
2026-05-12 8:21 ` Dan Carpenter
2026-05-12 13:54 ` Breno Leitao
0 siblings, 2 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2026-05-12 0:16 UTC (permalink / raw)
To: Dan Carpenter
Cc: kernel-janitors, Linux Trace Kernel, linux-kernel, Breno Leitao
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>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line
2026-05-12 0:16 ` Masami Hiramatsu
@ 2026-05-12 8:21 ` Dan Carpenter
2026-05-12 13:54 ` Breno Leitao
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2026-05-12 8:21 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: kernel-janitors, Linux Trace Kernel, linux-kernel, 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()
> > 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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line
2026-05-12 0:16 ` Masami Hiramatsu
2026-05-12 8:21 ` Dan Carpenter
@ 2026-05-12 13:54 ` Breno Leitao
1 sibling, 0 replies; 4+ messages in thread
From: Breno Leitao @ 2026-05-12 13:54 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Dan Carpenter, kernel-janitors, Linux Trace Kernel, linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-05-12 13:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 17:07 [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line Dan Carpenter
2026-05-12 0:16 ` Masami Hiramatsu
2026-05-12 8:21 ` Dan Carpenter
2026-05-12 13:54 ` Breno Leitao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.