Re: [PATCH v4 13/18] mm: Remove __alloc_pages_node()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch removes `__alloc_pages_node` because it is unsafe to use with `cpu_to_node()`, which can return `NUMA_NO_NODE` (-1) during CPU hotplug, but leaves the equivalent `__folio_alloc_node` untouched, which suffers from the exact same vulnerability.
--

commit 584eaae6f16d0eb43140d0b202665ffc9212c61e
Author: Brendan Jackman <jackmanb@xxxxxxxxxx>

mm: Remove __alloc_pages_node()

This commit removes __alloc_pages_node() since it is prone to memory
corruption on CPU hotplug if callers pass NUMA_NO_NODE from cpu_to_node().
Removing it leaves the __* variants strictly for internal memory management.

> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 01d6d2591f49e..3bf55a5f9143e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
[ ... ]
> @@ -256,21 +256,6 @@ static inline void warn_if_node_offline(int this_node, gfp_t gfp_mask)
>  	dump_stack();
>  }
>  
> -/*
> - * Allocate pages, preferring the node given as nid. The node must be valid and
> - * online. For more general interface, see alloc_pages_node().
> - */
> -static inline struct page *
> -__alloc_pages_node_noprof(int nid, gfp_t gfp_mask, unsigned int order)
> -{
> -	VM_BUG_ON(nid < 0 || nid >= MAX_NUMNODES);
> -	warn_if_node_offline(nid, gfp_mask);
> -
> -	return __alloc_pages_noprof(gfp_mask, order, nid, NULL);
> -}
> -
> -#define  __alloc_pages_node(...)		alloc_hooks(__alloc_pages_node_noprof(__VA_ARGS__))
> -
>  static inline
>  struct folio *__folio_alloc_node_noprof(gfp_t gfp, unsigned int order, int nid)
>  {

[Severity: High]
This isn't a bug introduced by this patch, but does __folio_alloc_node_noprof()
suffer from the exact same vulnerability that motivated removing
__alloc_pages_node_noprof()?

If NUMA_NO_NODE (-1) is passed here, it triggers VM_BUG_ON(nid < 0) or bypasses
it in non-debug builds, leading to out-of-bounds array accesses when evaluating
warn_if_node_offline() and NODE_DATA(-1).

Looking at net/sunrpc/svc.c, there is a path that passes the result of
cpu_to_node() directly to __folio_alloc_node() without checking for
NUMA_NO_NODE:

net/sunrpc/svc.c:svc_prepare_thread() {
    ...
    node = svc_pool_map_get_node(pool->sp_id);
    rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0, node);
    ...
}

During concurrent CPU hotplug, svc_pool_map_get_node() can return NUMA_NO_NODE.
Should __folio_alloc_node() be removed or fixed in this patch as well to fully
eliminate this class of bug?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-alloc-trylock-v4-0-0af8ff387e80@xxxxxxxxxx?part=13