Re: [PATCH 02/11] of: reserved_mem: reject reserved memory outside physical address range

From: Rob Herring <robh@kernel.org>
To: Chen Wandun <chenwandun1@gmail.com>
Cc: kexec@lists.infradead.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, loongarch@lists.linux.dev,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	akpm@linux-foundation.org, bhe@redhat.com, rppt@kernel.org,
	pasha.tatashin@soleen.com, pratyush@kernel.org,
	ruirui.yang@linux.dev, corbet@lwn.net, skhan@linuxfoundation.org,
	catalin.marinas@arm.com, will@kernel.org, chenhuacai@kernel.org,
	kernel@xen0n.name, pjw@kernel.org, palmer@dabbelt.com,
	aou@eecs.berkeley.edu, saravanak@kernel.org,
	chenwandun@lixiang.com, zhaomeijing@lixiang.com,
	everyzhao@126.com
Subject: Re: [PATCH 02/11] of: reserved_mem: reject reserved memory outside physical address range
Date: Tue, 5 May 2026 20:51:12 -0500	[thread overview]
Message-ID: <20260506015112.GA286568-robh@kernel.org> (raw)
In-Reply-To: <20260429065831.1510858-3-chenwandun@lixiang.com>

On Wed, Apr 29, 2026 at 02:58:22PM +0800, Chen Wandun wrote:
> early_init_dt_reserve_memory() does not validate whether the region
> falls within physical memory. If a device tree incorrectly specifies a
> reserved memory region outside the physical address range:
> 
>  - For the non-nomap path, memblock_reserve() blindly adds the region
>    to memblock.reserved, creating a stale entry that refers to
>    non-existent memory.
> 
>  - For the nomap path, memblock_mark_nomap() silently fails to match
>    any region in memblock.memory, but still returns success.
> 
> Add a memblock_overlaps_region() check at the entry of
> early_init_dt_reserve_memory() to reject such regions before any
> memblock operation takes place. This also simplifies the existing nomap
> guard: the original "overlaps && is_reserved" condition reduces to just
> "is_reserved", since the overlap with physical memory is already
> guaranteed by the new check.

While I agree, I suspect we already have cases abusing reserved-memory 
like this.

> 
> Signed-off-by: Chen Wandun <chenwandun@lixiang.com>
> Tested-by: Zhao Meijing <zhaomeijing@lixiang.com>
> ---
>  drivers/of/of_reserved_mem.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 9d1b0193864c..03c676052dab 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -112,14 +112,21 @@ static int fdt_fixup_reserved_mem_node(unsigned long node,
>  static int __init early_init_dt_reserve_memory(phys_addr_t base,
>  					       phys_addr_t size, bool nomap)
>  {
> +	if (!memblock_overlaps_region(&memblock.memory, base, size)) {
> +		phys_addr_t end = base + size - 1;
> +
> +		pr_warn("Reserved memory region %pa..%pa is outside of physical memory\n",
> +			&base, &end);
> +		return -EINVAL;
> +	}
> +
>  	if (nomap) {
>  		/*
>  		 * If the memory is already reserved (by another region), we
> -		 * should not allow it to be marked nomap, but don't worry
> -		 * if the region isn't memory as it won't be mapped.
> +		 * should not allow it to be marked nomap. The region being
> +		 * physical memory is guaranteed by the overlap check above.
>  		 */
> -		if (memblock_overlaps_region(&memblock.memory, base, size) &&
> -		    memblock_is_region_reserved(base, size))
> +		if (memblock_is_region_reserved(base, size))
>  			return -EBUSY;
>  
>  		return memblock_mark_nomap(base, size);
> -- 
> 2.43.0
> 