Re: [PATCH] page-alloc: fix initialization of cross-node regions

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 25/07/2022 17:18, Jan Beulich wrote:
> On 25.07.2022 18:05, Julien Grall wrote:
> > (Sorry for the formatting)
> No issues seen.
Good to know. I sent it from my phone and the gmail app used to mangle 
e-mails.
> > On Mon, 25 Jul 2022, 14:10 Jan Beulich, <jbeulich@xxxxxxxx> wrote:
> >
> > > Quite obviously to determine the split condition successive pages'
> > > attributes need to be evaluated, not always those of the initial page.
> > >
> > > Fixes: 72b02bc75b47 ("xen/heap: pass order to free_heap_pages() in heap
> > > init")
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > Part of the problem was already introduced in 24a53060bd37 ("xen/heap:
> > > Split init_heap_pages() in two"), but there it was still benign.
> > Is this because range will never cross numa node? How about the fake NUMA
> > node?
> No (afaict), because pages were still freed one by one (and hence node
> boundaries still wouldn't end up in the middle of a buddy).
So I agree that free_heap_pages() would be called with one page at the 
time. However, I think _init_heap_pages() would end up to be called with 
the full range.
So we would initialize the first node but not the others (if the range 
spans over multiple ones). Therefore, I think free_heap_pages() could 
dereference a NULL pointer.
Anyway, I would not expect anyone to only backport the patch to split 
_init_heap_pages() and... in any case you already committed it (which is 
fine given this is a major regression).
Cheers,

--
Julien Grall