From: Breno Leitao <leitao@debian.org>
To: sashiko-reviews@lists.linux.dev
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH 2/4] bootconfig: render embedded bootconfig as a kernel cmdline at build time
Date: Mon, 1 Jun 2026 10:52:41 -0700 [thread overview]
Message-ID: <ah3Fzof8H3vAvLwQ@gmail.com> (raw)
In-Reply-To: <20260527173037.5C9061F00A3D@smtp.kernel.org>
On Wed, May 27, 2026 at 05:30:36PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] The `tools/bootconfig` executable is built using the target architecture's compiler instead of the host architecture's compiler, causing an `Exec format error` during cross-compilation.
> - [Medium] The read-only data section `.init.rodata` is incorrectly marked as writable (`"aw"`).
> - [Medium] Missing 'clean' target hook for tools/bootconfig in the top-level Makefile.
>
> Pre-existing issues:
> - [High] Pointer arithmetic on a NULL pointer and passing an invalid pointer to `snprintf()` in `xbc_snprint_cmdline()`.
> --
>
> commit d68de9beacfcd22ccb081a670940139845227254
> Author: Breno Leitao <leitao@debian.org>
>
> bootconfig: render embedded bootconfig as a kernel cmdline at build time
>
> Add the build-time pipeline that renders the "kernel" subtree of
> CONFIG_BOOT_CONFIG_EMBED_FILE into a flat cmdline string and stashes
> it in .init.rodata as embedded_kernel_cmdline[].
>
> > diff --git a/Makefile b/Makefile
> > index d59f703f9797a..3ee259d00a9ae 100644
> > --- a/Makefile
> > +++ b/Makefile
> [ ... ]
> > @@ -1543,6 +1543,11 @@ prepare: tools/bpf/resolve_btfids
> > endif
> > endif
> >
> > +# lib/Makefile invokes tools/bootconfig to render the embedded bconf to cmdline.
> > +ifdef CONFIG_BOOT_CONFIG_EMBED_CMDLINE
> > +prepare: tools/bootconfig
> > +endif
>
> [Severity: Medium]
> Will this leave stale build artifacts behind? Since tools/bootconfig is
> wired into the prepare phase, should there also be a corresponding cleanup
> step added to the top-level clean target so that make clean removes the
> compiled binary and object files?
Correct, and this is the important one. lib/Makefile runs the binary on
the build host, so it must be built with the host compiler. Plain $(CC)
is the target cross-compiler under an ARCH=... build, so the tool comes
out as a target binary and fails to exec. I'll build it with $(HOSTCC),
matching how tools/objtool and tools/bpf/resolve_btfids do it
(HOST_OVERRIDES := CC="$(HOSTCC)" ...). This also fixes a standalone
cross-build of the tool.
> > diff --git a/lib/Makefile b/lib/Makefile
> > index 6e72d2c1cce71..9de0ac7732a21 100644
> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> [ ... ]
> > +quiet_cmd_render_cmdline = BCONF2C $@
> > + cmd_render_cmdline = \
> > + $(objtree)/tools/bootconfig/bootconfig -C $< > $@
> > +
> > +targets += embedded_cmdline.bin
> > +$(obj)/embedded_cmdline.bin: $(obj)/default.bconf $(objtree)/tools/bootconfig/bootconfig FORCE
> > + $(call if_changed,render_cmdline)
>
> [Severity: High]
> This isn't a bug introduced by this patch, but running bootconfig -C at
> build time exposes an issue in xbc_snprint_cmdline().
I will fix it in a different patch.
>
> When calculating the required buffer size, xbc_snprint_cmdline() is initially
> called with buf = NULL and size = 0. The first snprintf(NULL, 0, ...)
> returns the number of bytes that would be written, and the code then executes
> buf += ret. Doing pointer arithmetic on a NULL pointer is undefined behavior.
>
> In the next loop iteration, this invalid pointer is passed as the first
> argument to snprintf(), which can trigger host UBSan failures or
> FORTIFY_SOURCE aborts during the kernel build.
>
> > diff --git a/lib/embedded-cmdline.S b/lib/embedded-cmdline.S
> > new file mode 100644
> > index 0000000000000..7e2e1d81af962
> > --- /dev/null
> > +++ b/lib/embedded-cmdline.S
> > @@ -0,0 +1,16 @@
> [ ... ]
> > + .section .init.rodata, "aw"
>
> [Severity: Medium]
> Does this mistakenly mark the read-only section as writable?
>
> The "w" flag instructs the assembler and linker to make the section writable,
> violating the expectation for .rodata and potentially bypassing memory
> protections. Should this use "a" or the __INITRODATA macro instead?
Agreed -- the "w" flag has no business on rodata. I'll drop it to "a"
(read-only, allocatable) in lib/embedded-cmdline.S.
>
> > diff --git a/tools/bootconfig/Makefile b/tools/bootconfig/Makefile
> > index 90eb47c9d8de6..4e82fd9553cde 100644
> > --- a/tools/bootconfig/Makefile
> > +++ b/tools/bootconfig/Makefile
> [ ... ]
> > -all: $(ALL_PROGRAMS) test
> > +all: $(ALL_PROGRAMS)
> >
> > $(OUTPUT)bootconfig: main.c include/linux/bootconfig.h $(LIBSRC)
> > $(CC) $(filter %.c,$^) $(CFLAGS) $(LDFLAGS) -o $@
>
> [Severity: High]
> When cross-compiling the kernel, does this build bootconfig for the
> target architecture instead of the host?
>
> Kbuild exports CC as the target cross-compiler (e.g., aarch64-linux-gnu-gcc).
> Because this uses $(CC) rather than a host override like $(HOSTCC) (which
> objtool and resolve_btfids use), the tool is built for the target architecture.
>
> Later, lib/Makefile attempts to execute this tool on the build host to render
> the cmdline, which will fail with an Exec format error.
Correct, and this is the important one. lib/Makefile runs the binary on
the build host, so it must be built with the host compiler. Plain $(CC)
is the target cross-compiler under an ARCH=... build, so the tool comes
out as a target binary and fails to exec. I'll build it with $(HOSTCC),
matching how tools/objtool and tools/bpf/resolve_btfids do it
(HOST_OVERRIDES := CC="$(HOSTCC)" ...). This also fixes a standalone
cross-build of the tool.
next prev parent reply other threads:[~2026-06-01 17:52 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-27 16:41 [PATCH 0/4] bootconfig: embed kernel.* cmdline at build time Breno Leitao
2026-05-27 16:41 ` [PATCH 1/4] bootconfig: return 0 from xbc_snprint_cmdline() for a leaf root Breno Leitao
2026-05-27 17:05 ` sashiko-bot
2026-06-01 17:46 ` Breno Leitao
2026-05-27 16:41 ` [PATCH 2/4] bootconfig: render embedded bootconfig as a kernel cmdline at build time Breno Leitao
2026-05-27 17:30 ` sashiko-bot
2026-06-01 17:52 ` Breno Leitao [this message]
2026-05-27 16:41 ` [PATCH 3/4] bootconfig: add xbc_prepend_embedded_cmdline() helper Breno Leitao
2026-05-27 16:41 ` [PATCH 4/4] x86/setup: prepend embedded bootconfig cmdline before parse_early_param Breno Leitao
2026-05-27 18:07 ` sashiko-bot
2026-05-28 15:15 ` [PATCH 0/4] bootconfig: embed kernel.* cmdline at build time Masami Hiramatsu
2026-05-28 16:14 ` Breno Leitao
2026-06-01 17:56 ` 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=ah3Fzof8H3vAvLwQ@gmail.com \
--to=leitao@debian.org \
--cc=bpf@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.