Re: [PATCH v4 8/8] mm/mempolicy: read the zoneref slot once in mempolicy_slab_node()

[Vlastimil Babka (SUSE)]

On 10/28/26 22:19, Gregory Price (Meta) wrote:
> For a bind or preferred-many policy, mempolicy_slab_node() tests the
> first matching slot's zone for NULL, then reads the slot again to look
> up the zone's node. Offlining a zone's last block rebuilds the
> zonelists in place, so the second read can return the terminator:
> 
>   CPU 0                             CPU 1
>   mempolicy_slab_node()
>     zonelist_zone(slot 4)
>       = node 2 NORMAL
>                                     offline_pages()
>                                       build_all_zonelists()
>                                         slot 4 = NULL
>     zonelist_node_idx(slot 4)
>       zone_to_nid(NULL)
> 
> Current GCC merges the two reads, but nothing requires it. With
> mm/mempolicy.o built with -fno-inline -fno-ipa-sra, holding a bound
> task's slab_strict_numa allocation between the reads while a movable
> block earlier in the zonelist goes offline oopses every time:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000060
>   RIP: 0010:zone_to_nid+0x0/0x10
>   Call Trace:
>    kmem_cache_alloc_noprof+0x294/0x610
>    do_getname+0x34/0x230
> 
> mempolicy_slab_node() and zonelist_node_idx() are tail calls here, so
> neither appears in the trace.
> 
> Read the slot once and use that zone.
> 
> Fixes: 11cd8638c37f ("mm, page_alloc: remove stop_machine from build_all_zonelists")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>

> ---
>  mm/mempolicy.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 0884c629670d..cd63c3e2c029 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2176,6 +2176,7 @@ unsigned int mempolicy_slab_node(void)
>  	case MPOL_PREFERRED_MANY:
>  	{
>  		struct zoneref *z;
> +		struct zone *zone;
>  
>  		/*
>  		 * Follow bind policy behavior and start allocation at the
> @@ -2186,7 +2187,8 @@ unsigned int mempolicy_slab_node(void)
>  		zonelist = &NODE_DATA(node)->node_zonelists[ZONELIST_FALLBACK];
>  		z = first_zones_zonelist(zonelist, highest_zoneidx,
>  							&policy->nodes);
> -		return zonelist_zone(z) ? zonelist_node_idx(z) : node;
> +		zone = zonelist_zone(z);
> +		return zone ? zone_to_nid(zone) : node;
>  	}
>  	case MPOL_LOCAL:
>  		return node;