The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Dan Carpenter <error27@gmail.com>
Cc: kernel-janitors@vger.kernel.org,
	Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
	linux-kernel@vger.kernel.org, Breno Leitao <leitao@debian.org>
Subject: Re: [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line
Date: Tue, 12 May 2026 09:16:38 +0900	[thread overview]
Message-ID: <20260512091638.8b95253ab022d7dabf473465@kernel.org> (raw)
In-Reply-To: <af4YTUrDM-ciyoa-@stanley.mountain>

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>

       reply	other threads:[~2026-05-12  0:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <af4YTUrDM-ciyoa-@stanley.mountain>
2026-05-12  0:16 ` Masami Hiramatsu [this message]
2026-05-12  8:21   ` [bug report] bootconfig: init: Allow admin to use bootconfig for kernel command line Dan Carpenter
2026-05-12 13:54   ` Breno Leitao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260512091638.8b95253ab022d7dabf473465@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=error27@gmail.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox