[PATCH v2 1/2] fdt_support: bound serialN alias length before copying to stack

[Simon Glass]

Hi Aristo,

On 2026-05-26T02:09:13, Aristo Chen <aristo.chen at canonical.com> wrote:
> fdt_support: validate dma-ranges length in fdt_get_dma_range
>
> fdt_get_dma_range() fetches the dma-ranges property with fdt_getprop()
> and checks only that the length is non-zero before reading one full
> entry from it. The entry size depends on na, pna and ns cells returned
> by count_cells, which come from the parent buses in the devicetree.
> A dma-ranges property shorter than (na + pna + ns) * sizeof(u32) bytes
> causes fdt_read_number() and fdt_translate_dma_address() to read past
> the end of the property within the FDT blob, an out-of-bounds read of
> attacker-influenced data when the OS devicetree is not signature
> verified.
>
> Reject the property when its length is smaller than one full entry and
> return -EINVAL, matching the existing failure paths in this function.
> Use debug() rather than printf() for the rejection text so that
> production builds do not pay any .text or .rodata growth for the new
> diagnostic.
>
> Signed-off-by: Aristo Chen <aristo.chen at canonical.com>
>
> boot/fdt_support.c | 7 +++++++
>  1 file changed, 7 insertions(+)

> diff --git a/boot/fdt_support.c b/boot/fdt_support.c
> @@ -1640,6 +1640,13 @@ int fdt_get_dma_range(const void *blob, int node, phys_addr_t *cpu,
> +     if (len < (int)((na + pna + ns) * sizeof(*ranges))) {
> +             debug("%s: dma-ranges too short for %s\n", __func__,
> +                   fdt_get_name(blob, node, NULL));
> +             ret = -EINVAL;
> +             goto out;
> +     }

The bound looks correct: fdt_read_number() consumes na cells, the
translation reads pna cells from ranges + na and the size read covers
ns cells, so one full entry is na + pna + ns cells.

As a follow-up, dm_test_dma_ranges in test/dm/read.c already exercises
dev_get_dma_range() and the -ENOENT path. Adding a node with a
truncated dma-ranges to the sandbox DT and checking for -EINVAL would
cover the new path. What do you think?

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon