Re: [PATCH] memblock: don't touch memblock arrays when memblock_free() is called late

[Mike Rapoport <rppt@kernel.org>]

On Wed, May 13, 2026 at 05:47:34AM -0700, Breno Leitao wrote:
> On Wed, May 13, 2026 at 01:51:22PM +0300, Mike Rapoport wrote:
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> > 
> > When memblock_free() is called after memblock_discard() on architectures
> > that don't select ARCH_KEEP_MEMBLOCK, it tries to update memblock.reserved
> > that was already discarded and it causes use-after-free, for example
> > 
> > [    8.514775] BUG: KASAN: use-after-free in memblock_isolate_range+0x4ac/0x650
> > [    8.514775] Read of size 8 at addr ffff88a07fe6a000 by task swapper/0/1
> > [    8.514775] Call Trace:
> > [    8.514775]  <TASK>
> > [    8.514775]  kasan_report+0xb2/0x1b0
> > [    8.514775]  memblock_isolate_range+0x4ac/0x650
> > [    8.514775]  memblock_phys_free+0xc4/0x190
> > [    8.514775]  housekeeping_late_init+0x257/0x280
> > [    8.514775]  do_one_initcall+0xaa/0x470
> > [    8.514775]  do_initcalls+0x1b4/0x1f0
> > [    8.514775]  kernel_init_freeable+0x4b5/0x550
> > [    8.514775]  kernel_init+0x1c/0x150
> > [    8.514775]  ret_from_fork+0x5dc/0x8e0
> > [    8.514775]  ret_from_fork_asm+0x1a/0x30
> > [    8.514775]  </TASK>
> > 
> > Make sure memblock_free() updates memblock.reserved only when called early
> > enough or when ARCH_KEEP_MEMBLOCK is enabled.
> > 
> > Reported-by: Waiman Long <longman@redhat.com>
> > Reported-by: Breno Leitao <leitao@debian.org>
> > Closes: https://lore.kernel.org/all/20260505051821.1107133-1-longman@redhat.com
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > Tested-by: Waiman Long <longman@redhat.com>
> 
> Tested-by: Breno Leitao <leitao@debian.org>

Thanks!
 
> Don't you want a Fixes: tag?

Right,
Fixes: 87ce9e83ab8b ("memblock, treewide: make memblock_free() handle late freeing")
 
> > @@ -989,13 +989,15 @@ void __init_memblock memblock_free(void *ptr, size_t size)
> >  int __init_memblock memblock_phys_free(phys_addr_t base, phys_addr_t size)
> >  {
> >  	phys_addr_t end = base + size - 1;
> > -	int ret;
> > +	int ret = 0;
> >  
> >  	memblock_dbg("%s: [%pa-%pa] %pS\n", __func__,
> >  		     &base, &end, (void *)_RET_IP_);
> >  
> >  	kmemleak_free_part_phys(base, size);
> > -	ret = memblock_remove_range(&memblock.reserved, base, size);
> > +
> > +	if (!slab_is_available() || IS_ENABLED(CONFIG_ARCH_KEEP_MEMBLOCK))
> > +		ret = memblock_remove_range(&memblock.reserved, base, size);
> >  
> >  	if (slab_is_available())
> >  		__free_reserved_area(base, base + size, -1);
> 
> given slab_is_available() is a cheap function, it is fine to call it
> switch in here.
> 
> Thanks for the fix!
> --breno
> 

-- 
Sincerely yours,
Mike.