linux-kernel - Re: [PATCH] mm: memblock: Fix arguments passed to memblock_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250428091333.35120-1-bercantu@amazon.de>
Date: Mon, 28 Apr 2025 09:13:33 +0000
From: "Bernardo C. Gutierrez Cantu" <bercantu@...zon.de>
To: <akpm@...ux-foundation.org>
CC: <bercantu@...zon.de>, <dwmw2@...radead.org>,
	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>, <lkp@...el.com>,
	<rppt@...nel.org>, <yajun.deng@...ux.dev>
Subject: Re: [PATCH] mm: memblock: Fix arguments passed to memblock_set_node()

On Sat, 26 Apr 2025 03:05:39 +0200 Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Fri, 25 Apr 2025 10:20:03 +0000 "Bernardo C. Gutierrez Cantu" <bercantu@...zon.de> wrote:
> 
> > memblock_set_node() receives a `base` and a `size` arguments, but we are
> > passing the `start` and `end` of the memory regions when iterating over
> > them in memmap_init_reserved_pages() to set their node ids.
> > 
> > This results in the function setting the node ids for the reserved memory
> > regions in `[base, base + base + size)` instead of `[base, base + size)`.
> > 
> > Pass `start` and `size`, so that we iterate over the correct range.
> > 
> > Fixes: 61167ad5fecd ("mm: pass nid to reserve_bootmem_region()")
> > 
> > ...
> >
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -2196,7 +2196,7 @@ static void __init memmap_init_reserved_pages(void)
> >  		if (memblock_is_nomap(region))
> >  			reserve_bootmem_region(start, end, nid);
> >  
> > -		memblock_set_node(start, end, &memblock.reserved, nid);
> > +		memblock_set_node(start, region->size, &memblock.reserved, nid);
> >  	}
> >  
> >  	/*
> 
> What were the runtime effects of this bug?

I found the bug via code introspection while trying to learn how the boot time
memory allocation of Linux worked. I was not actively pursuing to fix a runtime
issue that I could then prove was fixed by this.

But I see that in most cases this bug should be mostly benign (which could
explain why it was not caught before). In kernels compiled with CONFIG_NUMA, the
memblock_set_node() function would iterate over more memblock regions and
potentially set incorrect node ids, which would then be overwritten with the
correct node ids once the caller function memmap_init_reserved_pages() iterates
over the next regions. So the resultant node ids after initialization should
still be correct.

It could still impact the length of the mm init process on kernel boot, but I
did not measure the improvement, as it is highly dependent on the memory
configuration system, and in my specific system under test, it was neglible.

Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597