Re: [PATCH v4] net: mvneta: re-enable percpu interrupt on resume

[sashiko-bot]

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

Pre-existing issues:
- [High] Unsafe call to mvneta_set_rx_mode() without netif_addr_lock_bh() in PM resume.
- [High] Race condition in mvneta_open() and mvneta_resume() with CPU hotplug causing double napi_enable().
--

commit bf4b34826006aca50b2726e755de72b6f2a7c330
Author: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>

net: mvneta: re-enable percpu interrupt on resume

On Marvell MPIC platforms (Armada 370/XP/38x), mvneta uses a percpu IRQ 
disable/enable scheme for NAPI. This commit fixes an issue where the MPIC 
per-CPU interrupt stays masked permanently after suspend by unmasking it 
unconditionally during the resume path.

> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 0c061fb0ed072..744d6585a949d 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -5900,6 +5900,9 @@ static int mvneta_resume(struct device *device)
>  	rtnl_unlock();
>  	mvneta_set_rx_mode(dev);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to call 
mvneta_set_rx_mode() here without holding netif_addr_lock_bh()?

The mvneta_set_rx_mode() function iterates over the net_device's multicast 
list:

drivers/net/ethernet/marvell/mvneta.c:mvneta_set_rx_mode() {
	...
			if (!netdev_mc_empty(dev)) {
				netdev_for_each_mc_addr(ha, dev) {
					mvneta_mcast_addr_set(pp, ha->addr,
							      pp->rxq_def);
				}
			}
	...
}

Since rtnl_unlock() is called immediately before this, it looks like this 
unprotected iteration could race with concurrent softirqs handling IGMP/MLD 
packets. Could this corrupt the list and lead to use-after-free crashes or 
infinite loops?

>  
> +	if (!pp->neta_armada3700)
> +		on_each_cpu(mvneta_percpu_enable, pp, true);
> +

[Severity: High]
This is a pre-existing issue, but does the resume path have a race condition 
with CPU hotplug?

In functions like mvneta_open() and mvneta_resume(), the device is added to 
the CPU hotplug state via cpuhp_state_add_instance_nocalls() without 
preventing concurrent hotplug events (e.g., via cpus_read_lock()).

If a CPU hotplug event triggers concurrently, mvneta_cpu_online() could call 
napi_enable_locked() for the new CPU:

drivers/net/ethernet/marvell/mvneta.c:mvneta_cpu_online() {
	...
	napi_enable_locked(&port->napi);
	...
}

If mvneta_start_dev() then runs and blindly iterates over all online CPUs 
calling napi_enable(), could this result in a double napi_enable() on the 
same NAPI instance and trigger a BUG_ON() panic in the core NAPI code?

>  	return 0;
>  }
> #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622074350.1666290-1-yun.zhou@xxxxxxxxxxxxx?part=1