Re: [PATCH v6 09/10] of: reserved_mem: mark /memreserve/ entries as MEMBLOCK_NODUMP

[Rob Herring <robh@kernel.org>]

On Wed, Sep 02, 2026 at 09:02:11AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] FDT /memreserve/ entries marked as MEMBLOCK_NODUMP can be later freed to the buddy allocator, causing active kernel memory to be silently omitted from crash dumps.
> - [Medium] 64-bit FDT reserve entries are checked for overlap in 64-bit space but implicitly truncated to phys_addr_t when marked as NODUMP, potentially corrupting dumpable regions on 32-bit systems.
> --
> 
> commit f6e6bcd14fb6f4946f258fc34089bbbdcd52f624
> Author: Wandun Chen <chenwandun@lixiang.com>
> 
> of: reserved_mem: mark /memreserve/ entries as MEMBLOCK_NODUMP
> 
> This patch marks /memreserve/ entries in the device tree as MEMBLOCK_NODUMP
> so they are excluded from kernel crash dumps. It explicitly leaves regions
> that overlap with a dumpable region 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;
> > +
> > +	if (!IS_ENABLED(CONFIG_CRASH_DUMP))
> > +		return;
> > +
> > +	if (!fdt)
> > +		return;
> > +
> > +	for (n = 0; ; n++) {
> > +		int i;
> > +
> > +		if (fdt_get_mem_rsv(fdt, n, &base, &size))
> > +			break;
> > +		if (!size)
> > +			break;
> > +
> > +		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]
> Will this corrupt dumpable regions on 32-bit systems?
> 
> base and size are u64, so the overlap check evaluates safely in 64-bit
> space. However, memblock_mark_nodump() takes phys_addr_t. On 32-bit
> systems, could a 64-bit FDT reserve entry > 4GB correctly pass the overlap
> check above, but then get implicitly truncated to a 32-bit address when
> passed to memblock_mark_nodump(), incorrectly marking a lower 32-bit
> physical address as NODUMP without having checked it for overlap?

Unlikely. Such a system would probably enable LPAE or equivalent.
 
> [Severity: High]
> Can this cause active kernel memory to be silently omitted from crash dumps?
> 
> If the bootloader specifies a /memreserve/ entry for memory that the kernel
> later frees (such as the initrd), this memory is returned to the buddy
> allocator.
> 
> Since the MEMBLOCK_NODUMP flag is never cleared, will this cause reused
> pages to be incorrectly skipped and omitted from the crash dump later on?

This seems like an issue. Both the FDT and initrd are typically in 
/memreserve/ and can be moved.

Rob