Re: [PATCH v5 11/18] 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] In `xpc_init_mq_node()`, the loop iterates over CPUs but mistakenly passes the `nid` (node ID) variable to `xpc_create_gru_mq_uv()` instead of the `cpu` iterator.
--

commit 60dc55f94c78f2b8722b9d132aa85a4603393bdd
Author: Brendan Jackman <jackmanb@xxxxxxxxxx>

sgi-xp: Use higher-level allocator API

The difference between __alloc_pages_node() and alloc_pages_node() is
that the latter allows you to pass NUMA_NO_NODE.

The former is going away and the latter works fine here so switch over.

No functional change intended.

> diff --git a/drivers/misc/sgi-xp/xpc_uv.c b/drivers/misc/sgi-xp/xpc_uv.c
> index 772c787268932..79c2f00ed4d70 100644
> --- a/drivers/misc/sgi-xp/xpc_uv.c
> +++ b/drivers/misc/sgi-xp/xpc_uv.c
> @@ -170,9 +170,8 @@ 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 isn't a bug introduced by this patch, but while looking at how the cpu
argument is handled here in xpc_create_gru_mq_uv(), I noticed an issue in
its caller.

In xpc_init_mq_node():

	for_each_cpu(cpu, cpumask_of_node(nid)) {
		xpc_activate_mq_uv =
			xpc_create_gru_mq_uv(XPC_ACTIVATE_MQ_SIZE_UV, nid,
					     XPC_ACTIVATE_IRQ_NAME,
					     xpc_handle_activate_IRQ_uv);

Is it possible that the node ID (nid) is mistakenly passed into
xpc_create_gru_mq_uv() instead of the cpu iterator? This also happens a few
lines later when allocating xpc_notify_mq_uv.

Passing a node ID where a CPU index is expected will cause per-CPU lookups
like uv_cpu_to_blade_id() and cpu_to_node() to access data for the wrong CPU.
If the node ID exceeds nr_cpu_ids, this could also cause out of bounds memory
accesses.

> -	page = __alloc_pages_node(nid,
> -				      GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
> -				      pg_order);
> +	page = alloc_pages_node(nid, GFP_KERNEL | __GFP_ZERO | __GFP_THISNODE,
> +				pg_order);
>  	if (page == NULL) {
>  		dev_err(xpc_part, "xpc_create_gru_mq_uv() failed to alloc %d "
>  			"bytes of memory on nid=%d for GRU mq\n", mq_size, nid);

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