On 31 Mar 2022, at 4:57, Vlastimil Babka wrote: > On 3/31/22 02:10, Zi Yan wrote: >> On 30 Mar 2022, at 19:48, Zi Yan wrote: >> >>> On 30 Mar 2022, at 19:03, Linus Torvalds wrote: >>> >>>> On Wed, Mar 30, 2022 at 3:12 PM Zi Yan wrote: >>>>> >>>>> Fixes: 1dd214b8f21c ("mm: page_alloc: avoid merging non-fallbackable pageblocks with others") >>>> >>>> Oh, btw - should this perhaps be backported further back than that >>>> alleged "fixes" commit? >>>> >>>> It does look like maybe the problem potentially existed before too, >>>> and was just much harder to trigger. >>>> >>>> That said, google doesn't find any other reports that look like >>>> Steven's oops, so maybe it really never happened and backporting isn't >>>> called for. >>>> >>>> Or possibly my google-fu is just bad. >>>> >>> >>> There might not be any issue with the original code because this bug >>> could only be triggered when CONFIG_FLATMEM and CONFIG_MEMORY_ISOLATION >>> are both set, which never happens, since CONFIG_MEMORY_ISOLATION >>> depends on CONFIG_SPARSEMEM. > > Good point. Which means unset_migratetype_isolate() that Linus pointed > out, is currently also safe as it's a CONFIG_MEMORY_ISOLATION code. > We could still implement the suggested page_find_buddy() wrapper using > page_is_buddy() internally, as well as the cleanup of __free_one_page(), > but it's not urgent. > Sure. Will do that. >>> By checking Steven's boot log, it should be PFN 0x21ee00 that triggers >>> the bug, since the physical memory range ends at PFN 0x21edff. >>> PFN 0x21ee00 is 2MB aligned instead of MAX_ORDER-1 (4MB) aligned. >>> The original code assumes all physical memory ranges are at least >>> MAX_ORDER-1 aligned, which is true when CONFIG_SPARSEMEM is set >>> (CONFIG_MEMORY_ISOLATION depends on it), since CONFIG_SPARSEMEM >>> allocates pageblock_flags array (the NULL-deferenced bitmap points >>> to) at section size granularity (128MB > 4MB). However, CONFIG_FLATMEM >>> does not do this. It allocates pageblock_flags array at the exact size >>> of the physical memory. So checking 0x21ee00 will not cause NULL >>> dereferencing when CONFIG_MEMORY_ISOLATION is set and the original >>> if statement can be true. For the record, the actual situation is that struct page of PFN 0x21ee00 is allocated but uninitialized. In CONFIG_FLATMEM, pageblock flags are stored in zone->pageblock_flags. When reading PFN 0x21ee00's pageblock flags, the page's zone is determined to be ZONE_MOVABLE, since page->flags is -1UL and page_zonenum() is 3. The system does not have ZONE_MOVABLE, so zone->pageblock_flags is NULL and reading it caused NULL pointer dereferencing. >>> >>> Now I am wondering if the page_is_buddy() check is correct for >>> CONFIG_FLATMEM. Is mem_map allocation aligned to MAX_ORDER-1 >>> or just the present physical memory range? Is PageBuddy(0x21ee00) >>> accessing some random memory location? >> >> OK. mem_map seems to be MAX_ORDER-1 aligned, so there is no >> problem with PageBuddy(0x21ee00). > > Yeah mem_map has to be in all config variants, otherwise buddy merging > would have been blowing up in page_is_buddy() even prior to all the > "sometimes avoid merging pageblock" changes. >> >> -- >> Best Regards, >> Yan, Zi -- Best Regards, Yan, Zi