Hi He,
On 2026-10-09T15:10:05, He Yong <[email protected]> wrote:
> bootstd: Add a bootmeth for scripts in raw partitions
>
> Some disk layouts store a U-Boot script image directly in a named
> partition, without a filesystem. The existing script bootmeth cannot
> find these images, so booting them requires a board-specific bootcmd.
>
> Add an optional script_raw bootmeth which matches a partition label and
> reads the image from the first block. Determine the image size from its
> header and check it against the partition size and a configurable limit
> before allocating and reading the complete image.
>
> Support legacy script images and FIT scripts with embedded data. Check
> legacy length-table bounds and validate the complete FIT before passing
> it to image_locate_script(). Reject FIT external data to avoid following
> references outside the loaded buffer. Use the existing script verification
> policy and share the script boot handler to set the device environment
> variables and execute the image.
Rejecting external data is unexpected behaviour, but I suppose it is
fair enough, since scripts are small and we don't expect any other
images in the FIT.
>
> Document the configuration and disk layout, and add sandbox tests for
> legacy and FIT execution, invalid images, size limits, partition labels
Philosophically speaking, it would be nice to extern the existing
bootmeth_script - did you consider that.
> [...]
>
> MAINTAINERS | 1 +
> boot/Kconfig | 26 ++++
> boot/Makefile | 1 +
> boot/bootmeth_script.c | 4 +-
> boot/bootmeth_script_raw.c | 202
> ++++++++++++++++++++++++++++++
> doc/develop/bootstd/index.rst | 1 +
> doc/develop/bootstd/script_raw.rst | 45 +++++++
> doc/device-tree-bindings/bootmeth.txt | 1 +
> include/bootmeth.h | 12 ++
> test/py/tests/test_bootmeth_script_raw.py | 97 ++++++++++++++
> 10 files changed, 388 insertions(+), 2 deletions(-)
> diff --git a/boot/bootmeth_script_raw.c b/boot/bootmeth_script_raw.c
> @@ -0,0 +1,202 @@
> + lengths = buf + pos;
> + len = fdt32_to_cpu(*lengths);
> + do {
> + if (size - pos < sizeof(*lengths))
> + return -EINVAL;
> + pos += sizeof(*lengths);
> + } while (*lengths++);
> + if (!len || len > size - pos)
> + return -EINVAL;
This duplicates the legacy parsing in image_locate_script(), which
takes a size but ignores it, so its 'while (*data++);' loop can run
off the end of the buffer. Please can you add the bounds checks to
image_locate_script() instead, in a separate patch? Then the existing
script bootmeth and the 'source' command get the same protection, and
this bootmeth can just call image_locate_script(). What do you think?
> diff --git a/boot/bootmeth_script_raw.c b/boot/bootmeth_script_raw.c
> @@ -0,0 +1,202 @@
> + if (size < pos + 2 * sizeof(*lengths) ||
> + image_get_data_size(hdr) != size - pos ||
> + !image_check_hcrc(hdr))
> + return -EINVAL;
The caller computes 'size' from image_get_data_size() and has already
checked the header CRC, so neither check can fail. The same applies to
the 'fdt_totalsize(buf) != size' check in the FIT branch. Please drop
these, so it is clearer which checks matter.
> diff --git a/boot/bootmeth_script_raw.c b/boot/bootmeth_script_raw.c
> @@ -0,0 +1,202 @@
> + if (size < sizeof(struct fdt_header)) {
> + ret = -EINVAL;
> + goto out;
> + }
This only applies to FIT but also runs for legacy images. Please move
it into the FIT branch above.
> diff --git a/boot/bootmeth_script_raw.c b/boot/bootmeth_script_raw.c
> @@ -0,0 +1,202 @@
> + if (!bflow->blk || !bflow->part)
> + return -ENOENT;
> + desc = dev_get_uclass_plat(bflow->blk);
> + ret = part_get_info(desc, bflow->part, &info);
> + if (ret)
> + return ret;
Please use log_msg_ret() for error returns here and on the 'goto out'
paths, as other bootmeths do. With so many rejection reasons, it helps
when debugging why a partition is skipped.
> diff --git a/doc/develop/bootstd/script_raw.rst
> b/doc/develop/bootstd/script_raw.rst
> @@ -0,0 +1,45 @@
> +Raw Script Bootmeth
> +==================
The underline is one character short, so Sphinx warns and 'make
htmldocs' fails in CI.
> diff --git a/test/py/tests/test_bootmeth_script_raw.py
> b/test/py/tests/test_bootmeth_script_raw.py
> @@ -0,0 +1,97 @@
> [email protected]('sandbox')
> [email protected]('bootmeth_script_raw')
No defconfig enables CONFIG_BOOTMETH_SCRIPT_RAW, so CI always skips
this test. Please can you enable it for sandbox? Check that the
existing bootflow/bootmeth tests still pass, since they count the
auto-bound bootmeths. The test also needs
@pytest.mark.requiredtool('sgdisk'), as in test_ab.py.
bootmeths should be tested in C, in test/boot/bootflow.c, with a disk
image created by test/py/img/ - that is quicker than restarting U-Boot
for each case and avoids the manual 'bind bootstd 0
bootmeth_script_raw' step. See the docs here if you haven't already:
https://docs.u-boot-project.org/en/latest/develop/tests_writing.html
Regards,
Simon
