Hi He,

On 2026-10-26T14:02:15, He Yong <[email protected]> wrote:
> test: bootstd: Cover scripts in raw partitions on sandbox
>
> Enable script_raw for sandbox and add a GPT disk with valid legacy and
> FIT scripts, as well as malformed and unsupported images. Bind the
> bootmeth inside the new C test so existing bootmeth lists and bootflow
> tests, including bootflow scan -G coverage, remain unchanged.
>
> Introduce test/py/img/ as a package for disk-image generators. Existing
> C-test image setup lives in test/py/tests/test_ut.py; keep the raw-script
> case definitions and image generation in the new module and call it
> from the existing test_ut_dm_init_bootstd() initialization.
>
> Allow fs_helper.setup_image() to accept an sfdisk partition specification
> and reuse it for the named GPT partitions. This avoids a dependency on
> sgdisk or a separate initialization test. Annotate each expected C error
> with its Python case name to keep the two lists in sync.

The fs_helper change affects every caller of setup_image(), since it
switches from printf | sfdisk to run(). Please can you put it in its
own patch, before this one?

>
> Signed-off-by: He Yong <[email protected]>
>
>  arch/sandbox/dts/test.dts  |  8 ++++
>  configs/sandbox_defconfig  |  1 +
>  test/boot/bootflow.c       | 81 ++++++++++++++++++++++++++++++++++++++
>  test/py/img/__init__.py    |  2 +
>  test/py/img/script_raw.py  | 98 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  test/py/tests/fs_helper.py | 19 +++++----
>  test/py/tests/test_ut.py   |  3 ++
>  7 files changed, 205 insertions(+), 7 deletions(-)

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1948,4 +1949,84 @@ static int bootdev_hunt_fallthrough(struct 
> unit_test_state *uts)
> +             0,              /* legacy */
> +             0,              /* fit */
> +             -EINVAL,        /* external */
> +             -EINVAL,        /* bad_crc */
> +             -E2BIG,         /* too_large */
> +             -ENOENT,        /* wrong_name */
> +             -ENOEXEC,       /* empty */
> +             -EINVAL,        /* bad_header */
> +             -E2BIG,         /* partition_size */
> +             -EINVAL,        /* bad_fit */

The raw bootmeth turns every image_locate_script() failure into
-EINVAL, so the external, bad_crc and bad_fit cases cannot tell which
check rejected the image. For example, if the external-data check in
patch 1 were dropped, the external case would still get -EINVAL from
somewhere else. Since the test already uses UTF_CONSOLE, it would be
worth checking the message for each case too, e.g. 'External FIT
script data is not supported' and 'Bad FIT image format'.

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1948,4 +1949,84 @@ static int bootdev_hunt_fallthrough(struct 
> unit_test_state *uts)
> +             bootflow_free(&bflow);
> +     }
> +
> +     return 0;

Please can you check that the scan ends after the last partition (i.e.
bootflow_scan_next() returns -ENODEV)? Otherwise an unexpected extra
bootflow would go unnoticed.

> diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
> @@ -1948,4 +1949,84 @@ static int bootdev_hunt_fallthrough(struct 
> unit_test_state *uts)
> +     ut_assertok(prep_mmc_bootdev(uts, "mmc12", false, &old_order));
> +     ut_assertok(bootstd_get_priv(&std));
> +     std->bootdev_order = order;

prep_mmc_bootdev() also binds the script bootmeth and sets its own
order {'mmc2', 'mmc1', 'mmc12'}, and you then override both straight
away. It would be clearer to bind mmc12 directly with lists_bind_fdt()
and set the order once, as bootflow_bls_prefix() and bootflow_rauc()
do.

> diff --git a/test/py/img/script_raw.py b/test/py/img/script_raw.py
> @@ -0,0 +1,98 @@
> +    if case == 'bad_crc':
> +        data[-1] ^= 1
> +    elif case == 'too_large':
> +        struct.pack_into('>I', data, 12, 2 * 1024 * 1024)
> +    elif case == 'empty':
> +        data = bytearray(512)
> +    if case == 'too_large':
> +        struct.pack_into('>I', data, 4, 0)
> +        struct.pack_into('>I', data, 4, zlib.crc32(data[:64]))
> +    return data

The corruption is split between script_image() and
setup_script_raw_image() for no obvious reason, and the header-CRC
fix-up is repeated in both. Please can you put all the per-case
changes in one place, with the CRC fix-up in a small helper? For
'empty' it would also be simpler to return early, before running
mkimage.

Regards,
Simon

Reply via email to