Re: [PATCH v7 8/9] of: reserved_mem: mark /memreserve/ entries as MEMBLOCK_NODUMP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] of/fdt: silent u64 truncation in fdt_mark_memreserve_nodump causes nodump false positives
--

commit 070ed699ada4d456618a640c6ad541abbec98f95
Author: Wandun Chen <chenwandun@lixiang.com>

of: reserved_mem: mark /memreserve/ entries as MEMBLOCK_NODUMP

The /memreserve/ entries are memory reservations made by the bootloader
or firmware, their contents are not needed for kernel crash analysis. This
commit marks them MEMBLOCK_NODUMP to omit them from the vmcore, while leaving
overlapping dumpable regions unmarked.

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index f29a07e8fb23e..a42cf6eac55b5 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
[ ... ]
> @@ -251,6 +251,39 @@ static void __init __rmem_check_for_overlap(void)
>  	}
>  }
>  
> +void __init fdt_mark_memreserve_nodump(void)
> +{
> +	u64 base, size;
> +	int n;
> +	const void *fdt = initial_boot_params;
[ ... ]
> +		for (i = 0; i < reserved_mem_count; i++) {
> +			struct reserved_mem *rmem = &reserved_mem[i];
> +
> +			if (rmem->dumpable &&
> +			    base < (u64)rmem->base + (u64)rmem->size &&
> +			    (u64)rmem->base < base + size)
> +				break;
> +		}
> +		if (i == reserved_mem_count)
> +			memblock_mark_nodump(base, size);

[Severity: Medium]
Is there a risk of silent truncation here when passing u64 arguments to
memblock_mark_nodump() on 32-bit architectures?

The u64 base and size variables are passed directly to memblock_mark_nodump()
which takes phys_addr_t arguments. On a 32-bit architecture where phys_addr_t
is 32-bit, if the device tree contains a /memreserve/ entry with an address
greater than 4GB, fdt_mark_memreserve_nodump() reads it as a 64-bit value.

It performs a 64-bit overlap check against dumpable regions in reserved_mem.
Because the upper 32 bits differ, it concludes there is no overlap. It then
calls memblock_mark_nodump(), where the compiler silently truncates base to 32
bits.

Could this incorrectly mark the lower 32-bit address as MEMBLOCK_NODUMP,
mistakenly excluding any dumpable region located there that the overlap
check was intended to protect?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922084256.1515554-1-chenwandun1@gmail.com?part=8