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 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.