* [PATCH net 1/2] net: mctp: usb: fix race between urb completion and rx_retry cancellation
2026-06-05 7:24 [PATCH net 0/2] net: mctp: usb: minor fixes for MCTP over USB transport driver Jeremy Kerr
@ 2026-06-05 7:24 ` Jeremy Kerr
2026-06-05 7:24 ` [PATCH net 2/2] net: mctp: usb: don't fail mctp_usb_rx_queue on a deferred submission Jeremy Kerr
1 sibling, 0 replies; 3+ messages in thread
From: Jeremy Kerr @ 2026-06-05 7:24 UTC (permalink / raw)
To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, Jeremy Kerr
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()
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.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
drivers/net/mctp/mctp-usb.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index 3b5dff144177..cf6f6a93a451 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -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;
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH net 2/2] net: mctp: usb: don't fail mctp_usb_rx_queue on a deferred submission
2026-06-05 7:24 [PATCH net 0/2] net: mctp: usb: minor fixes for MCTP over USB transport driver Jeremy Kerr
2026-06-05 7:24 ` [PATCH net 1/2] net: mctp: usb: fix race between urb completion and rx_retry cancellation Jeremy Kerr
@ 2026-06-05 7:24 ` Jeremy Kerr
1 sibling, 0 replies; 3+ messages in thread
From: Jeremy Kerr @ 2026-06-05 7:24 UTC (permalink / raw)
To: Matt Johnston, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, Jeremy Kerr
In the ndo_open path, a deferred queue open will report a failure, and
so the netdev will not be ndo_stop()ed, leaving us with the rx_retry
work potentially pending.
Don't report a deferred queue as an error, as we are still operational.
This means we use the ndo_stop() path for future cleanup, which handles
rx_retry_work cancellation.
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
drivers/net/mctp/mctp-usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/mctp/mctp-usb.c b/drivers/net/mctp/mctp-usb.c
index cf6f6a93a451..fade65f2f269 100644
--- a/drivers/net/mctp/mctp-usb.c
+++ b/drivers/net/mctp/mctp-usb.c
@@ -154,7 +154,7 @@ static int mctp_usb_rx_queue(struct mctp_usb *mctp_usb, gfp_t gfp)
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;
+ return 0;
}
static void mctp_usb_in_complete(struct urb *urb)
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread