Re: [PATCH net-next v4 5/6] net: usb: pegasus: Move long delayed work on system_dfl_long_wq

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usbnet: pegasus: missing locking around multi-step MII USB control messages
--

commit b4ecccdd796761b12d0bedd35e1c982cb24bc36a
Author: Marco Crivellari <marco.crivellari@suse.com>

net: usb: pegasus: Move long delayed work on system_dfl_long_wq

This patch shifts the delayed work for carrier checks from the per-CPU
system_long_wq to the unbound system_dfl_long_wq. This allows the scheduler
to optimize task placement for long running tasks.

Cc: Petko Manolov <petkan@nucleusys.com>
Cc: linux-usb@vger.kernel.org
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
> index aba1a640fc268..ed3f8d555c2a0 100644
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -1118,8 +1118,9 @@ static void check_carrier(struct work_struct *work)
>  	pegasus_t *pegasus = container_of(work, pegasus_t, carrier_check.work);
>  	set_carrier(pegasus->net);
>  	if (!(pegasus->flags & PEGASUS_UNPLUG)) {
> -		queue_delayed_work(system_long_wq, &pegasus->carrier_check,
> -			CARRIER_CHECK_DELAY);
> +		queue_delayed_work(system_dfl_long_wq,
> +				   &pegasus->carrier_check,
> +				   CARRIER_CHECK_DELAY);
>  	}
>  }

[Severity: High]
Does this code risk data corruption in the PHY registers?

The delayed work check_carrier() runs asynchronously in a workqueue and calls
set_carrier(), which performs multi-step USB control transfers without holding
rtnl_lock() or any internal driver lock.

Concurrently, userspace ethtool operations or ioctls (which do hold rtnl_lock)
can read or write PHY registers.

Since the operations interleave on the USB bus without serialization, concurrent
execution could overlap the hardware address and control commands, corrupting
the PHY state:

drivers/net/usb/pegasus.c:__mii_op() {
    ...
    set_register(p, PhyCtrl, 0);
    set_registers(p, PhyAddr, sizeof(data), data);
    set_register(p, PhyCtrl, (indx | cmd));
    ...
}

Would it be possible to add locking around these MII hardware accesses to
prevent interleaving when called concurrently?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012095452.135828-1-marco.crivellari@suse.com?part=5