All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Florian Bezdeka" <florian.bezdeka@siemens.com>
Cc: linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC 3/3] net: stmmac: Migrate IRQ balancing to cpumask_local_spread()
Date: Thu, 20 Aug 2026 14:30:47 +0000	[thread overview]
Message-ID: <20260820143048.3DCB41F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260819-flo-net-7-2-make-stmmac-default-affinity-aware-v1-3-3f79a99cadaf@siemens.com>

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 <florian.bezdeka@siemens.com>

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

  parent reply	other threads:[~2026-08-20 14:30 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19 14:30 [PATCH RFC 0/3] genirq: Allow drivers to respect userspace IRQ affinities Florian Bezdeka
2026-08-19 14:30 ` [PATCH RFC 1/3] cpumask: Honor irq_default_affinity in cpumask_local_spread() Florian Bezdeka
2026-08-19 18:38   ` Yury Norov
2026-08-20 16:09     ` Sebastian Andrzej Siewior
2026-08-20 14:30   ` sashiko-bot
2026-08-19 14:30 ` [PATCH RFC 2/3] genirq: Honor existing IRQ affinities when setting affinity hints Florian Bezdeka
2026-08-20 14:30   ` sashiko-bot
2026-08-20 16:41   ` Jakub Kicinski
2026-08-19 14:30 ` [PATCH RFC 3/3] net: stmmac: Migrate IRQ balancing to cpumask_local_spread() Florian Bezdeka
2026-08-19 18:45   ` Yury Norov
2026-08-20 14:30   ` sashiko-bot [this message]
2026-08-19 18:28 ` [PATCH RFC 0/3] genirq: Allow drivers to respect userspace IRQ affinities Yury Norov
2026-08-19 23:54 ` Andrew Lunn
2026-08-20  0:10   ` Andrew Lunn
2026-08-20 15:12 ` Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260820143048.3DCB41F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=florian.bezdeka@siemens.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.