Re: [PATCH v3 03/11] of: reserved_mem: avoid post-init UAF when alloc_reserved_mem_array() fails

From: Rob Herring <robh@kernel.org>
To: Wandun Chen <chenwandun1@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, loongarch@lists.linux.dev,
	linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
	kexec@lists.infradead.org, iommu@lists.linux.dev,
	zhaomeijing@lixiang.com, catalin.marinas@arm.com,
	will@kernel.org, chenhuacai@kernel.org, kernel@xen0n.name,
	pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	alex@ghiti.fr, saravanak@kernel.org, akpm@linux-foundation.org,
	bhe@redhat.com, rppt@kernel.org, pasha.tatashin@soleen.com,
	pratyush@kernel.org, ruirui.yang@linux.dev,
	m.szyprowski@samsung.com, robin.murphy@arm.com,
	quic_obabatun@quicinc.com
Subject: Re: [PATCH v3 03/11] of: reserved_mem: avoid post-init UAF when alloc_reserved_mem_array() fails
Date: Tue, 2 Jun 2026 11:24:50 -0500	[thread overview]
Message-ID: <20260602162450.GA442759-robh@kernel.org> (raw)
In-Reply-To: <20260527032917.3385849-4-chenwandun1@gmail.com>

On Wed, May 27, 2026 at 11:29:09AM +0800, Wandun Chen wrote:
> From: Wandun Chen <chenwandun@lixiang.com>
> 
> The global pointer 'reserved_mem' continues to reference the
> reserved_mem_array which lives in __initdata if
> alloc_reserved_mem_array() fails. of_reserved_mem_lookup() is
> exported for post-init use, that would dereference freed memory
> and trigger a use-after-free.
> 
> So reset reserved_mem_count to 0 when alloc_reserved_mem_array()
> fails.
> 
> Fixes: 00c9a452a235 ("of: reserved_mem: Add code to dynamically allocate reserved_mem array")

Fixes should come first in a series.

> Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
> ---
>  drivers/of/of_reserved_mem.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 313cbc57aa45..6d479381ff1f 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -69,29 +69,31 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>   * the initial static array is copied over to this new array and
>   * the new array is used from this point on.
>   */
> -static void __init alloc_reserved_mem_array(void)
> +static bool __init alloc_reserved_mem_array(void)
>  {
>  	struct reserved_mem *new_array;
>  	size_t alloc_size, copy_size, memset_size;
>  
> +	if (!total_reserved_mem_cnt)
> +		return true;
> +
>  	alloc_size = array_size(total_reserved_mem_cnt, sizeof(*new_array));
>  	if (alloc_size == SIZE_MAX) {
>  		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -EOVERFLOW);
> -		return;
> +		goto fail;
>  	}
>  
>  	new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
>  	if (!new_array) {
>  		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -ENOMEM);
> -		return;
> +		goto fail;
>  	}
>  
>  	copy_size = array_size(reserved_mem_count, sizeof(*new_array));
>  	if (copy_size == SIZE_MAX) {
>  		memblock_free(new_array, alloc_size);
> -		total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
>  		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -EOVERFLOW);

These prints could be moved to 'fail'. Perhaps instead of just printing 
an error value, you can return the error value instead of boolean.

If you respin just this patch, I can pick it up for 7.2.

Rob