Re: [PATCH RFC 3/3] net: stmmac: Migrate IRQ balancing to cpumask_local_spread()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The dynamically allocated cpumask 'affinity' is stored as a pointer in all IRQ descriptors via irq_set_affinity_and_hint() and then immediately freed, leading to a use-after-free when the affinity hint is read.
--

--- Patch [3]: [PATCH RFC 3/3] net: stmmac: Migrate IRQ balancing to cpumask_local_spread() ---
commit 490c157da6154ac1da229e94e4e1cd92226addfa
Author: Florian Bezdeka <[email protected]>

net: stmmac: Migrate IRQ balancing to cpumask_local_spread()

This commit migrates the IRQ balancing mechanism from num_online_cpus() to
cpumask_local_spread() to provide NUMA locality and honor the default SMP
affinity mask, avoiding isolated CPUs.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index a71f0df263785..949ced7e46d28 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3826,11 +3826,16 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
>  	struct stmmac_priv *priv = netdev_priv(dev);
>  	struct stmmac_msi *msi = priv->msi;
>  	enum request_irq_err irq_err;
> +	cpumask_var_t affinity;
>  	int irq_idx = 0;
>  	char *int_name;
> +	int node;
>  	int ret;
>  	int i;
>  
> +	if (!zalloc_cpumask_var(&affinity, GFP_KERNEL))
> +		return -ENOMEM;
> +
[ ... ]
> @@ -3935,8 +3941,10 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
>  			irq_idx = i;
>  			goto irq_error;
>  		}
> -		irq_set_affinity_hint(msi->rx_irq[i],
> -				      cpumask_of(i % num_online_cpus()));
> +
> +		cpumask_clear(affinity);
> +		cpumask_set_cpu(cpumask_local_spread(i, node), affinity);
> +		irq_set_affinity_and_hint(msi->rx_irq[i], affinity);
>  	}
>  
>  	/* Request Tx MSI irq */
> @@ -3959,13 +3967,18 @@ static int stmmac_request_irq_multi_msi(struct net_device *dev)
>  			irq_idx = i;
>  			goto irq_error;
>  		}
> -		irq_set_affinity_hint(msi->tx_irq[i],
> -				      cpumask_of(i % num_online_cpus()));
> +
> +		cpumask_clear(affinity);
> +		cpumask_set_cpu(cpumask_local_spread(i, node), affinity);
> +		irq_set_affinity_and_hint(msi->tx_irq[i], affinity);
>  	}
>  
> +	free_cpumask_var(affinity);

[Severity: Critical]
Does freeing this cpumask here leave dangling pointers in the IRQ
descriptors?

When calling irq_set_affinity_and_hint() in the loops above, it stores
the literal pointer to the local affinity mask in the descriptor,
rather than making a copy.

Since free_cpumask_var() is called unconditionally on the success path,
the mask memory is freed while the IRQ descriptor still points to it.

Could this lead to a use-after-free, for example, if a user reads the
affinity hint later?

kernel/irq/proc.c:irq_affinity_hint_proc_show() {
    ...
    cpumask_copy(mask, desc->affinity_hint);
    ...
}

> +
>  	return 0;
>  
>  irq_error:
> +	free_cpumask_var(affinity);
>  	stmmac_free_irq(dev, irq_err, irq_idx);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819-flo-net-7-2-make-stmmac-default-affinity-aware-v1-0-3f79a99cadaf@siemens.com?part=3