Re: [PATCH v1 0/3] fdt_support: validate property lengths in chosen and dma-range fixups

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2319 bytes --]

On Mon, May 25, 2026 at 01:26:22PM +0000, Aristo Chen wrote:

> boot/fdt_support.c contains a number of helpers that fix up the kernel
> devicetree handed to the OS during bootm/booti. Several of those
> helpers consume fdt_getprop() results without validating the returned
> length against the per-entry size implied by the surrounding cell-count
> arithmetic. When the OS devicetree is not signature-verified, for
> example an unsigned FIT, a DT loaded from $fdtaddr or $fdtcontroladdr,
> or a DT supplied over a network boot, the property is
> attacker-influenced and the missing checks turn into out-of-bounds
> reads or writes on the FDT blob and on stack buffers.
> 
> The first patch targets fdt_fixup_stdout(). The function copies the
> value of /aliases/serialN into a fixed 256-byte stack buffer before
> publishing it as /chosen/linux,stdout-path, but does not check that
> the property fits. The patch rejects an oversized property with a
> warning and -FDT_ERR_NOSPACE so the unbounded memcpy cannot run.
> 
> The second patch addresses fdt_get_dma_range(). The function reads one
> full dma-ranges entry of (na + pna + ns) * sizeof(u32) bytes after
> checking only that the returned length is non-zero. A dma-ranges
> property shorter than one entry causes the subsequent fdt_read_number()
> and fdt_translate_dma_address() calls to read past the property within
> the FDT blob. The patch validates the length against one full entry
> and returns -EINVAL when the property is too short, matching the
> existing failure paths in this function.
> 
> The third patch is an unrelated cleanup. A handful of printf call
> sites in fdt_fixup_memory_banks, __of_translate_address and
> fdt_get_dma_range still use the gcc-specific __FUNCTION__ identifier
> while the rest of the file already uses the C99-standard __func__.
> The patch converts the remaining occurrences for consistency with the
> rest of the file.
> 
> Aristo Chen (3):
>   fdt_support: bound serialN alias length before copying to stack
>   fdt_support: validate dma-ranges length in fdt_get_dma_range

I'm a little concerned about the potential size growth of adding
warnings in these cases, can you please check how much the growth is and
move them to debug() if it's non-trivial? Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]