get_page_from_freelist() and alloc_pages_bulk_noprof() reread ac->preferred_zoneref->zone at every use. Offlining a zone's last block rebuilds the zonelists in place, and the shrunk list's NULL terminator can land on the preferred slot mid-walk: CPU 0 CPU 1 get_page_from_freelist(GFP_KERNEL | __GFP_THISNODE) zone = NORMAL (slot 1) offline_pages() build_all_zonelists() [MOVABLE, NORMAL, NULL] -> [NORMAL, NULL] rmqueue(zonelist_zone(slot 1), zone) zone_statistics(NULL, zone) BUG: kernel NULL pointer dereference, address: 0000000000000060 RIP: 0010:get_page_from_freelist+0x6a5/0x15f0 Call Trace: __alloc_frozen_pages_noprof+0x125/0x330 Read the slot once. Zones are never freed, so a stale zone is safe, and a NULL slot fails the same way as walking off the end. Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists") Cc: stable@vger.kernel.org Signed-off-by: Gregory Price (Meta) --- mm/page_alloc.c | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 52533992ea16..24106c881ec2 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -3794,12 +3794,21 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags, { struct zoneref *z; struct zone *zone; + struct zone *preferred_zone; struct pglist_data *last_pgdat = NULL; bool last_pgdat_dirty_ok = false; bool no_fallback; bool skip_kswapd_nodes = nr_online_nodes > 1; bool skipped_kswapd_nodes = false; + /* + * A concurrent zonelist rebuild can NULL or replace this slot. Zones + * are never freed, so reading it once keeps a usable zone. + */ + preferred_zone = READ_ONCE(ac->preferred_zoneref->zone); + if (!preferred_zone) + return NULL; + retry: /* * Scan zonelist, looking for a zone with enough free. @@ -3846,7 +3855,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags, } if (no_fallback && !defrag_mode && nr_online_nodes > 1 && - zone != zonelist_zone(ac->preferred_zoneref)) { + zone != preferred_zone) { int local_nid; /* @@ -3854,7 +3863,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags, * fragmenting fallbacks. Locality is more important * than fragmentation avoidance. */ - local_nid = zonelist_node_idx(ac->preferred_zoneref); + local_nid = zone_to_nid(preferred_zone); if (zone_to_nid(zone) != local_nid) { alloc_flags &= ~ALLOC_NOFRAGMENT; goto retry; @@ -3916,7 +3925,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags, goto try_this_zone; if (!node_reclaim_enabled() || - !zone_allows_reclaim(zonelist_zone(ac->preferred_zoneref), zone)) + !zone_allows_reclaim(preferred_zone, zone)) continue; if (!node_reclaim(zone->zone_pgdat, gfp_mask, order)) @@ -3929,7 +3938,7 @@ get_page_from_freelist(gfp_t gfp_mask, unsigned int order, int alloc_flags, } try_this_zone: - page = rmqueue(zonelist_zone(ac->preferred_zoneref), zone, order, + page = rmqueue(preferred_zone, zone, order, gfp_mask, alloc_flags, ac->migratetype); if (page) { prep_new_page(page, order, gfp_mask, alloc_flags); @@ -5184,6 +5193,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid, { struct page *page; struct zone *zone; + struct zone *preferred_zone; struct zoneref *z; struct per_cpu_pages *pcp; struct list_head *pcp_list; @@ -5232,6 +5242,11 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid, if (!prepare_alloc_pages(gfp, 0, preferred_nid, nodemask, &ac, &gfp, &alloc_flags)) goto out; + /* Read once; see get_page_from_freelist(). */ + preferred_zone = READ_ONCE(ac.preferred_zoneref->zone); + if (!preferred_zone) + goto failed; + /* Find an allowed local zone that meets the low watermark. */ z = ac.preferred_zoneref; for_next_zone_zonelist_nodemask(zone, z, ac.highest_zoneidx, ac.nodemask) { @@ -5242,8 +5257,8 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid, continue; } - if (nr_online_nodes > 1 && zone != zonelist_zone(ac.preferred_zoneref) && - zone_to_nid(zone) != zonelist_node_idx(ac.preferred_zoneref)) { + if (nr_online_nodes > 1 && zone != preferred_zone && + zone_to_nid(zone) != zone_to_nid(preferred_zone)) { goto failed; } @@ -5251,7 +5266,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid, retry_this_zone: mark = wmark_pages(zone, alloc_flags & ALLOC_WMARK_MASK) + nr_pages - nr_populated; if (zone_watermark_fast(zone, 0, mark, - zonelist_zone_idx(ac.preferred_zoneref), + zone_idx(preferred_zone), alloc_flags, gfp)) { break; } @@ -5310,7 +5325,7 @@ unsigned long alloc_pages_bulk_noprof(gfp_t gfp, int preferred_nid, pcp_spin_unlock(pcp); __count_zid_vm_events(PGALLOC, zone_idx(zone), nr_account); - zone_statistics(zonelist_zone(ac.preferred_zoneref), zone, nr_account); + zone_statistics(preferred_zone, zone, nr_account); out: return nr_populated; -- 2.56.0