Re: [PATCH v3 11/16] sgi-xp: Use higher-level allocator API

[sashiko-bot]

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

Pre-existing issues:
- [High] xpc_init_mq_node() incorrectly passes the node ID (nid) instead of the loop variable (cpu) to xpc_create_gru_mq_uv().
--

commit 364bc05827d5201ce117b2c0d83f21a9b64f85f3
Author: Brendan Jackman <jackmanb@xxxxxxxxxx>

sgi-xp: Use higher-level allocator API

This commit replaces __alloc_pages_node() with alloc_pages_node() in
xpc_create_gru_mq_uv(). The __alloc_pages_node() function is being phased
out, and alloc_pages_node() works identically here while allowing
NUMA_NO_NODE if it were needed.

> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
> index 772c787268932..aacff70204241 100644
> --- a/drivers/misc/sgi-xp/xpc_uv.c
> +++ b/drivers/misc/sgi-xp/xpc_uv.c
> @@ -170,7 +170,7 @@ xpc_create_gru_mq_uv(unsigned int mq_size, int cpu, char *irq_name,
>  	mq->mmr_blade = uv_cpu_to_blade_id(cpu);
>  
>  	nid = cpu_to_node(cpu);

[Severity: High]
This is a pre-existing issue, but while reviewing this code, I noticed a
potential out-of-bounds access issue.

In xpc_init_mq_node(), the node ID (nid) is incorrectly passed instead of the
loop variable (cpu) to xpc_create_gru_mq_uv():

drivers/misc/sgi-xp/xpc_uv.c:xpc_init_mq_node() {
    ...
		xpc_activate_mq_uv =
			xpc_create_gru_mq_uv(XPC_ACTIVATE_MQ_SIZE_UV, nid,
					     XPC_ACTIVATE_IRQ_NAME,
					     xpc_handle_activate_IRQ_uv);
    ...
}

Because xpc_create_gru_mq_uv() expects a valid CPU index, passing a NUMA node
ID means the cpu parameter here contains a node ID. Since NUMA node IDs and
CPU IDs are in different ID spaces, cpu_to_node() will access the numa_node
per-CPU variable using a node ID instead of a CPU ID.

If the node ID is greater than or equal to nr_cpu_ids, will this result in an
out-of-bounds access causing a kernel panic during module load?

Even if it is within bounds, does this result in routing interrupts and
allocating memory using the topology of the wrong CPU?

> -	page = __alloc_pages_node(nid,
> +	page = alloc_pages_node(nid,
>  				      GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
>  				      pg_order);
>  	if (page == NULL) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-alloc-trylock-v3-0-57bef0eadbc2@xxxxxxxxxx?part=11