Catcrafts/milos-linux
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

net: mctp: usb: fix race between urb completion and rx_retry cancellation

Browse source 
It's possible that sequencing between setting ->stopped and cancelling
the rx_retry work (in ndo_stop) could leave us with an urb queued:

    T1: ndo_stop                  T2: rx_retry_work
    ------------                  ----------------
                                  LD: ->stopped => false
    ST: ->stopped <= true
    usb_kill_urb()
                                  mctp_usb_rx_queue()
                                    usb_submit_urb()
    cancel_delayed_work_sync()

That urb completion can then re-schedule rx_retry_work.

Strenghen the sequencing between the stop (preventing another requeue)
and the cancel by updating both atomically under a new rx lock. After
setting ->rx_stopped, and cancelling pending work, we know that the
requeue cannot occur, so all that's left is killing any pending urb.

Fixes: 0791c0327a ("net: mctp: Add MCTP USB transport driver")
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
Link: https://patch.msgid.link/20260608-dev-mctp-usb-rx-requeue-v2-1-29a3aa507609@codeconstruct.com.au
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
This commit is contained in:
Jeremy Kerr 2026-06-08 09:25:40 +08:00 • committed by Paolo Abeni
commit 54665dce98
1 changed files with 17 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

26
drivers/net/mctp/mctp-usb.c
View file

@ -22,7 +22,6 @@
struct mctp_usb {
struct usb_device *usbdev;
struct usb_interface *intf;
bool stopped;
struct net_device *netdev;
@ -32,6 +31,9 @@ struct mctp_usb {
struct urb *tx_urb;
struct urb *rx_urb;
/* enforces atomic access to rx_stopped and requeuing the retry work */
spinlock_t rx_lock;
bool rx_stopped;
struct delayed_work rx_retry_work;
};
@ -122,6 +124,7 @@ static const unsigned long RX_RETRY_DELAY = HZ / 4;
static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
{
unsigned long flags;
struct sk_buff *skb;
int rc;
@ -147,7 +150,10 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
return rc;
err_retry:
schedule_delayed_work(&mctp_usb->rx_retry_work, RX_RETRY_DELAY);
spin_lock_irqsave(&mctp_usb->rx_lock, flags);
if (!mctp_usb->rx_stopped)
schedule_delayed_work(&mctp_usb->rx_retry_work, RX_RETRY_DELAY);
spin_unlock_irqrestore(&mctp_usb->rx_lock, flags);
return rc;
}
@ -248,9 +254,6 @@ static void mctp_usb_rx_retry_work(struct work_struct *work)
struct mctp_usb *mctp_usb = container_of(work, struct mctp_usb,
rx_retry_work.work);
if (READ_ONCE(mctp_usb->stopped))
return;
mctp_usb_rx_queue(mctp_usb, GFP_KERNEL);
}
@ -258,7 +261,7 @@ static int mctp_usb_open(struct net_device *dev)
{
struct mctp_usb *mctp_usb = netdev_priv(dev);
WRITE_ONCE(mctp_usb->stopped, false);
WRITE_ONCE(mctp_usb->rx_stopped, false);
netif_start_queue(dev);
@ -268,17 +271,21 @@ static int mctp_usb_open(struct net_device *dev)
static int mctp_usb_stop(struct net_device *dev)
{
struct mctp_usb *mctp_usb = netdev_priv(dev);
unsigned long flags;
netif_stop_queue(dev);
/* prevent RX submission retry */
WRITE_ONCE(mctp_usb->stopped, true);
spin_lock_irqsave(&mctp_usb->rx_lock, flags);
mctp_usb->rx_stopped = true;
cancel_delayed_work(&mctp_usb->rx_retry_work);
spin_unlock_irqrestore(&mctp_usb->rx_lock, flags);
flush_delayed_work(&mctp_usb->rx_retry_work);
usb_kill_urb(mctp_usb->rx_urb);
usb_kill_urb(mctp_usb->tx_urb);
cancel_delayed_work_sync(&mctp_usb->rx_retry_work);
return 0;
}
@ -331,6 +338,7 @@ static int mctp_usb_probe(struct usb_interface *intf,
dev->netdev = netdev;
dev->usbdev = interface_to_usbdev(intf);
dev->intf = intf;
spin_lock_init(&dev->rx_lock);
usb_set_intfdata(intf, dev);
dev->ep_in = ep_in->bEndpointAddress;