Re: [PATCH] mm/mm_init: handle alloc_percpu failure in free_area_init_core_hotplug

[Mike Rapoport <rppt@kernel.org>]

On Tue, Jun 30, 2026 at 05:40:39PM -0400, Gregory Price wrote:
> We miss a failed allocation check for pgdat->per_cpu_nodestats, which
> results in a NULL deref when we offset into the per-cpu area.
> 
> Propagate -ENOMEM up the stack and leave per_cpu_nodestats pointing
> at boot_nodestats so a later online can retry the allocation.
> 
> hotadd_init_pgdat() returns NULL on failure, which __try_online_node()
> already maps to -ENOMEM.
> 
> Assisted-by: Sashiko:unknown-model

I suppose it's rather 

Reported-by: Sashiko <sashiko-bot@kernel.org>

> Fixes: 75ef71840539 ("mm, vmstat: add infrastructure for per-node vmstats")
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
>  include/linux/memory_hotplug.h |  2 +-
>  mm/memory_hotplug.c            |  3 ++-
>  mm/mm_init.c                   | 14 +++++++++++---
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 7c9d66729c60..f04b915678db 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -289,7 +289,7 @@ static inline void __remove_memory(u64 start, u64 size) {}
>  /* Default online_type (MMOP_*) when new memory blocks are added. */
>  extern enum mmop mhp_get_default_online_type(void);
>  extern void mhp_set_default_online_type(enum mmop online_type);
> -extern void __ref free_area_init_core_hotplug(struct pglist_data *pgdat);
> +extern int __ref free_area_init_core_hotplug(struct pglist_data *pgdat);

Nit: we are trying to get rid of 'extern's in the headers, even though it
makes the declarations inconsistent.
Can you please drop the extern since you anyway changing this?

>  extern int __add_memory(int nid, u64 start, u64 size, mhp_t mhp_flags);
>  extern int add_memory(int nid, u64 start, u64 size, mhp_t mhp_flags);
>  extern int add_memory_resource(int nid, struct resource *resource,
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 7ac19fab2263..8b137328dcf0 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1263,7 +1263,8 @@ static pg_data_t *hotadd_init_pgdat(int nid)
>  	pgdat = NODE_DATA(nid);
>  
>  	/* init node's zones as empty zones, we don't have any present pages.*/
> -	free_area_init_core_hotplug(pgdat);
> +	if (free_area_init_core_hotplug(pgdat))
> +		return NULL;
>  
>  	/*
>  	 * The node we allocated has no zone fallback lists. For avoiding
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index 306ea5c13f54..37fd64ce144d 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -1536,7 +1536,7 @@ void __init set_pageblock_order(void)
>   * NOTE: this function is only called during memory hotplug
>   */
>  #ifdef CONFIG_MEMORY_HOTPLUG
> -void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
> +int __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
>  {
>  	int nid = pgdat->node_id;
>  	enum zone_type z;
> @@ -1544,8 +1544,14 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
>  
>  	pgdat_init_internals(pgdat);
>  
> -	if (pgdat->per_cpu_nodestats == &boot_nodestats)
> -		pgdat->per_cpu_nodestats = alloc_percpu(struct per_cpu_nodestat);
> +	if (pgdat->per_cpu_nodestats == &boot_nodestats) {
> +		struct per_cpu_nodestat __percpu *p;
> +
> +		p = alloc_percpu(struct per_cpu_nodestat);
> +		if (!p)
> +			return -ENOMEM;
> +		pgdat->per_cpu_nodestats = p;
> +	}
>  
>  	/*
>  	 * Reset the nr_zones, order and highest_zoneidx before reuse.
> @@ -1583,6 +1589,8 @@ void __ref free_area_init_core_hotplug(struct pglist_data *pgdat)
>  		zone->present_pages = 0;
>  		zone_init_internals(zone, z, nid, 0);
>  	}
> +
> +	return 0;
>  }
>  #endif
>  
> -- 
> 2.53.0-Meta
> 

-- 
Sincerely yours,
Mike.