[PATCH 1/3] bootstd: Fix memleak on errors in bootmeth_setup_iter_order()

[Sam Protsenko]

On Fri, Feb 7, 2025 at 7:26 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sam,
>
> On Fri, 7 Feb 2025 at 18:11, Sam Protsenko <semen.protsenko at linaro.org> wrote:
> >
> > On Tue, Jan 14, 2025 at 7:14 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > On Sat, 11 Jan 2025 at 20:42, Sam Protsenko <semen.protsenko at linaro.org> wrote:
> > > >
> > > > Free memory allocated for 'order' (array of bootmeths) on error paths in
> > > > bootmeth_setup_iter_order() function.
> > > >
> > > > Fixes: c627cfc14c08 ("bootstd: Allow scanning for global bootmeths separately")
> > > > Fixes: 10d16faa436c ("bootstd: Detect empty bootmeth")
> > > > Signed-off-by: Sam Protsenko <semen.protsenko at linaro.org>
> > > > ---
> > > >  boot/bootmeth-uclass.c | 16 ++++++++++++----
> > > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > >
> > >
> > > Reviewed-by: Simon Glass <sjg at chromium.org>
> > >
> > > How about adding a test to check there is no leak? You can see
> > > test/lib/abuf.c for similar tests.
> >
> > That might be a good idea, and I already have the test code handy. But
> > frankly I wasn't able to successfully run the current bootstd test in
> > sandbox, it fails before it can get to memleak check I added. Firstly,
> > when I do:
> >
> >     $ make sandbox_defconfig
> >     $ make -j32
> >     $ ./u-boot -T -c "ut bootstd"
> >
> > it says:
> >
> >     MMC:   Can't map file 'mmc1.img': Invalid argument
> >
> > and I can see only empty (zero size) mmc1.img in my U-Boot source code
> > root dir. Then I was able to trigger its creation (I guess) by running
> > Python suite somehow, so it generated 20 MiB mmc1.img. But even with
> > that added, I hit this error:
> >
> >     test/boot/bootdev.c:218, bootdev_test_order():
> >         0 == bootflow_scan_first(((void *)0), ((void *)0), &iter, 0, &bflow):
> >         Expected 0x0 (0), got 0xffffffed (-19)
> >
> > I've read corresponding doc [1] ("Tests" section), but still don't
> > understand how can I successfully run the bootstd test locally. If you
> > can help me out, I can send the memleak test you mentioned in v2.
>
> You can run the bootstd tests with something like:
>
> test/py/test.py -B sandbox --build-dir /tmp/b/sandbox -k bootstd
>
> although it sounds like you have already figured that out. After you
> do it once (to create the images) you can run the C tests directly as
> you did above, but yes, sadly, some of the tests are affected by
> others (with bootstd). You could take a look if you like.
>

Is it possible to just apply this patch without a test? It's clearly
fixing a memleak. Not that I'm lazy or anything, I came up with a test
like below, but I don't want to send something that is quite hard to
test. If you think this test is really essential, I can send it once
bootstd tests are fixed and can be run in one command without any
extra non-documented work.

8<---------------------------------------------------------------------->8
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 9af947868707..f4746ee4fb61 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -194,6 +194,7 @@ static int bootdev_test_order(struct unit_test_state *uts)
 {
        struct bootflow_iter iter;
        struct bootflow bflow;
+       ulong start;

        test_set_skip_delays(true);

@@ -201,6 +202,8 @@ static int bootdev_test_order(struct unit_test_state *uts)
        bootstd_reset_usb();
        ut_assertok(run_command("usb start", 0));

+       start = ut_check_free();
+
        /*
         * First try the order set by the bootdev-order property
         * Like all sandbox unit tests this relies on the devicetree setting up
@@ -277,6 +280,9 @@ static int bootdev_test_order(struct unit_test_state *uts)
                        iter.dev_used[3]->name);
        bootflow_iter_uninit(&iter);

+       /* Check for memory leaks */
+       ut_assertok(ut_check_delta(start));
+
        return 0;
 }
 BOOTSTD_TEST(bootdev_test_order, UTF_DM | UTF_SCAN_FDT);
8<---------------------------------------------------------------------->8

> Regards,
> Simon
>
> >
> > Thanks!
> >
> > [1] doc/develop/bootstd/overview.rst