From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: Bradley Morgan <include@grrlz.net>,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
linux-trace-kernel@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH v2] lib/bootconfig: fix undefined behavior involving NULL pointer arithmetic
Date: Wed, 1 Jul 2026 08:26:11 +0900 [thread overview]
Message-ID: <20260701082611.6575e0f37541014f67830bdc@kernel.org> (raw)
In-Reply-To: <20260701075843.a308d7dadf327eda4015236b@kernel.org>
On Wed, 1 Jul 2026 07:58:43 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> On Tue, 30 Jun 2026 17:47:46 +0000
> Bradley Morgan <include@grrlz.net> wrote:
>
> > When xbc_snprint_cmdline() is called during the size-probing phase
> > (with buf = NULL and size = 0), the function computes the end pointer
> > as 'buf + size' (NULL + 0) and repeatedly advances 'buf' via 'buf += ret'.
> >
> > Under the C standard, performing pointer arithmetic on a NULL pointer is
> > undefined behavior. While harmless inside the kernel, this code is also
> > compiled into the userspace host tool 'tools/bootconfig', where host
> > compilers with UBSan or FORTIFY_SOURCE enabled abort the build when they
> > detect NULL pointer arithmetic.
> >
> > Fix this by guarding the pointer arithmetic so 'buf' is only advanced when
> > non-NULL, and track the running written length in a separate 'len' counter
> > for the return value (which cannot be recovered from pointer math when
> > 'buf' is NULL). The rest() helper and snprintf call sites are unchanged.
> >
> > Fixes: 51887d03aca1 ("bootconfig: init: Allow admin to use bootconfig for kernel command line")
> > Cc: stable@vger.kernel.org
> > Assisted-by: GLM:glm-5.2
> > Signed-off-by: Bradley Morgan <include@grrlz.net>
>
> Thanks for the fix!
> Let me pick this to bootconfig/fixes.
Sorry, I eventually decided to pick Breno's fix [1], because it fixes
the same issue earlier (in bootconfig/core) and has a well documented
comment on the code.
[1] https://lore.kernel.org/all/20260626-bootconfig_using_tools-v7-1-24ab72139c29@debian.org/
BTW, I decided to have several branches for bootconfig and probes.
bootconfig/core is a core development branch, which is the main branch.
The patches in this branch is for development, including new features
and fixes. (but fixes will be moved to */fixes soon.)
bootconfig/fixes is for a branch to manage fixes. This will be sent to
Linus soon (for urgent fix), or after releasing -rc.
bootconfig/for-next is for new features or cleanups, for preparing the
next merge window, and for merge test in linux-next.
If you make any patches, please check the bootconfig/core at first,
and check bootconfig/fixes for fix.
Note: The core is usually forcibly updated, actively rebased on top of
bootconfig/fixes. The for-next is not so frequently updated, but can
be forced update for fixing merge conflict etc. The fixes should be
solid, but if I made mistakes I will forcibly update it before sending
PR.
Thank you,
>
> Thank you,
>
> > ---
> > lib/bootconfig.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > Changes since v1:
> > - Got the big guns out! :) (see Assisted-by).
> > - Addressed review from Masami Hiramatsu and Breno Leitao.
> >
> > diff --git a/lib/bootconfig.c b/lib/bootconfig.c
> > index f445b7703fdd..c913259c80ce 100644
> > --- a/lib/bootconfig.c
> > +++ b/lib/bootconfig.c
> > @@ -427,8 +427,9 @@ static char xbc_namebuf[XBC_KEYLEN_MAX] __initdata;
> > int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
> > {
> > struct xbc_node *knode, *vnode;
> > - char *end = buf + size;
> > + char *end = buf ? buf + size : NULL;
> > const char *val, *q;
> > + size_t len = 0;
> > int ret;
> >
> > xbc_node_for_each_key_value(root, knode, val) {
> > @@ -442,7 +443,9 @@ int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
> > ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
> > if (ret < 0)
> > return ret;
> > - buf += ret;
> > + len += ret;
> > + if (buf)
> > + buf += ret;
> > continue;
> > }
> > xbc_array_for_each_value(vnode, val) {
> > @@ -456,11 +459,13 @@ int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
> > xbc_namebuf, q, val, q);
> > if (ret < 0)
> > return ret;
> > - buf += ret;
> > + len += ret;
> > + if (buf)
> > + buf += ret;
> > }
> > }
> >
> > - return buf - (end - size);
> > + return len;
> > }
> > #undef rest
> >
> > --
> > 2.53.0
> >
>
>
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
next prev parent reply other threads:[~2026-06-30 23:26 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-30 17:47 [PATCH v2] lib/bootconfig: fix undefined behavior involving NULL pointer arithmetic Bradley Morgan
2026-06-30 22:58 ` Masami Hiramatsu
2026-06-30 23:26 ` Masami Hiramatsu [this message]
2026-06-30 23:06 ` Masami Hiramatsu
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=20260701082611.6575e0f37541014f67830bdc@kernel.org \
--to=mhiramat@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=include@grrlz.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=stable@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