Re: [PATCH v5 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:
- [Critical] The patch removes `__alloc_pages_node()` to fix CPU hotplug bugs but completely misses the equivalent `__folio_alloc_node()` API, leaving it exposed with the same `NUMA_NO_NODE` vulnerability.
--

commit 4b93237c4920d8046053a32bcd0656e3236e97c9
Author: Brendan Jackman <jackmanb@xxxxxxxxxx>

mm: Remove __alloc_pages_node()

There were only a few users, which have been removed. The only advantage
of this API over alloc_pages_node() is avoiding a single conditional
branch.

> 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: Critical]
This is a pre-existing issue, but does __folio_alloc_node_noprof() suffer
from the exact same NUMA_NO_NODE vulnerability that you're fixing here?

This function lacks the fallback safety check for NUMA_NO_NODE that
alloc_pages_node() provides.

For example, looking at svc_prepare_thread() in net/sunrpc/svc.c, it calls
__folio_alloc_node() using a node ID that can be derived from cpu_to_node():

net/sunrpc/svc.c:svc_prepare_thread() {
    ...
	rqstp->rq_scratch_folio = __folio_alloc_node(GFP_KERNEL, 0, node);
	if (!rqstp->rq_scratch_folio)
		goto out_enomem;
    ...
}

If a CPU hotplug event causes the node to be NUMA_NO_NODE, couldn't this
trigger the same out-of-bounds array access into NODE_DATA() and result in
the memory corruption mentioned in the commit message?

Should __folio_alloc_node() also be removed or replaced with a safe
folio_alloc_node() wrapper?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@xxxxxxxxxx?part=13