Hi Simon,
On Sat, Sep 19, 2026 at 4:45 AM Simon Glass <[email protected]> wrote:
>
> Hi Aristo,
>
> On 2026-08-31T14:25:38, Aristo Chen <[email protected]> wrote:
> > test: boot: Check that rauc rejects over-long slot names
> >
> > Boot the RAUC bootflow after setting BOOT_ORDER to a 33-character slot
> > name, one more than fits the "BOOT_<slot>_LEFT" env-var name buffer. The
> > boot must now fail with -ENAMETOOLONG.
> >
> > Before the previous commit the name overflowed the stack buffer; as no
> > such variable exists the boot then failed with -ENOENT, hiding the
> > overflow.
> >
> > Also check the accepted side of the boundary: a 32-character name, the
> > longest that fits, must pass the length check and fail only with -ENOENT
> > because its BOOT_<slot>_LEFT variable does not exist.
> >
> > Signed-off-by: Aristo Chen <[email protected]>
> >
> > test/boot/bootflow.c | 23 +++++++++++++++++++++++
> > 1 file changed, 23 insertions(+)
>
> Since the boundary is boot_left_name(), please exercise the scan path
> too - distro_rauc_read_bootflow() calls it against
> CONFIG_BOOTMETH_RAUC_BOOT_ORDER, which is what a user is most likely
> to hit. The current test only reaches find_active_slot() via
> BOOT_ORDER at boot time.
During a scan the only names that reach boot_left_name() are the entries
of CONFIG_BOOTMETH_RAUC_BOOT_ORDER, a compile-time string ("A B" on
sandbox). The BOOT_ORDER entries seen at scan time go through get_slot()
in distro_rauc_scan_parts() instead, which marks the bootflow invalid
when a name does not match a configured slot, so no env-var name is
built from them. The test cannot change the Kconfig value at run time,
so covering the rejecting side of the scan path would need either a
dedicated sandbox configuration (which would require adapting or
excluding the existing rauc test, since the bootflow becomes invalid)
or a test hook that lets the scan path take an alternate default
order. The scan path's normal accepted-input case is exercised by every
scan in bootflow_rauc(), and it goes through the same helper the
boot-time test drives across the 32/33-char boundary. If you still
want the rejecting Kconfig scan path covered, I can add a dedicated
test configuration, although that seems disproportionate for this fix.
>
> > + /*
> > + * A 32-char slot name is the longest that fits. It must pass the
> > + * length check and fail only because BOOT_<slot>_LEFT does not exist.
> > + */
> > + ut_assertok(env_set("BOOT_ORDER",
> > "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"));
> > + ut_asserteq(1, run_command("bootflow boot", 0));
> > + ut_assert_nextline("** Booting bootflow 'mmc10.bootdev.whole' with
> > rauc");
> > + ut_assert_nextline("Boot failed (err=%d)", -ENOENT);
> > + ut_assert_console_end();
>
> > diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> > @@ -1520,6 +1520,29 @@ static int bootstd_images(struct unit_test_state
> > *uts)
> > ptr = map_sysmem(img->addr, 0);
> > ut_asserteq_strn("BOOT_IMAGE=/vmlinuz-5.15.0-121-generic root=", ptr);
> >
> > + /*
> > + * Boot with a BOOT_ORDER slot name (33 chars) that is too long for
> > the
> > + * "BOOT_<slot>_LEFT" env-var name buffer. It must be rejected rather
> > + * than overflow the stack.
> > + */
> > + ut_assertok(run_command("bootflow select 0", 0));
> > + ut_assert_console_end();
> > + ut_assertok(env_set("BOOT_ORDER", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> > B"));
> > + ut_asserteq(1, run_command("bootflow boot", 0));
> > + ut_assert_nextline("** Booting bootflow 'mmc10.bootdev.whole' with
> > rauc");
> > + ut_assert_nextline("Boot failed (err=%d)", -ENAMETOOLONG);
> > + ut_assert_console_end();
>
> This is in the wrong function. bootstd_images() scans
> mmc2/mmc1/mmc4/mmc5 and produces the four extlinux/script/cros
> bootflows listed just above; it never binds bootmeth_rauc or enables
> mmc10, so 'bootflow select 0' picks mmc1.bootdev.part_1 (extlinux_cfg)
> and the expected "** Booting bootflow 'mmc10.bootdev.whole' with rauc"
> line never appears. Please extend bootflow_rauc() (or add a sibling
> test that reuses its setup) so mmc10 is bound and rauc is in the
> bootmeth order before 'bootflow boot' runs. Also restore BOOT_ORDER
> (and the bootmeth/bootdev order) at the end so it does not leak into
> the next test.
The hunk you quote is not the one in the patch. As sent it is
@@ -1445,6 +1445,29 @@ static int bootflow_rauc(struct unit_test_state *uts)
so it does extend bootflow_rauc(), right after the scan and list that
leave the single rauc bootflow on mmc10 as index 0. The series is based
on next (base-commit 856a00aeb61 in the cover letter) because it builds
on the bootflow_rauc() additions from the "Fix crashes and memory leaks
on scan/boot error paths" series, which is only in next. I think the
hunk you saw comes from applying the patch to master: patch(1) accepts
it there with fuzz 2, landing inside bootstd_images(), which matches
your quote, while git am refuses it.
On the restore: bootflow_rauc() already sets BOOT_ORDER back to "A B"
(the value distro_rauc_read_bootflow() seeds from
CONFIG_BOOTMETH_RAUC_BOOT_ORDER) and restores the bootdev order before
returning, and UTF_DM gives every test a fresh driver-model root, so the
bootmeth order does not carry over either.
Looking into that last point did turn up a small pre-existing leak: the
array allocated by bootmeth_set_order() is never freed when the bootstd
device goes away, since bootstd_remove() frees bootdev_order but not
bootmeth_order. It affects every test that sets a bootmeth order (three
in bootflow.c alone) and real teardown too, so it seems better handled
as a separate one-line fix to bootstd_remove() than folded into this
series; I can send that patch.
I re-checked both patches on top of current next (951132236b9): they
apply cleanly and bootflow_rauc passes, so I will hold off on a resend
unless you or Tom prefer one.
>
> Regards,
> Simon
Best regards,
Aristo
