Linux USB
 help / color / mirror / Atom feed
* [PATCH] xhci: dbgtty: convert rx push tasklet to BH workqueue
@ 2026-10-08 12:06 Kun Luo
  2026-10-08 12:23 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Kun Luo @ 2026-10-08 12:06 UTC (permalink / raw)
  To: Mathias Nyman
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Lu Baolu, Kun Luo

The kernel is converting remaining tasklet users to BH workqueues so
softirq handling can go through the workqueue path. The dbc TTY rx
push path is a straightforward bottom-half: schedule from IRQ /
unthrottle, run the same work, kill on port teardown.

Replace the tasklet with system_bh_wq. Behavior stays in softirq
context, so existing locking is unchanged.

Compile-tested only.

Signed-off-by: Kun Luo <luokun485@gmail.com>
---
 drivers/usb/host/xhci-dbgcap.h |  2 +-
 drivers/usb/host/xhci-dbgtty.c | 17 +++++++++--------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h
index 5b18efb2c1ea..78e9b5e674a3 100644
--- a/drivers/usb/host/xhci-dbgcap.h
+++ b/drivers/usb/host/xhci-dbgcap.h
@@ -127,7 +127,7 @@ struct dbc_port {
 	struct list_head		read_pool;
 	struct list_head		read_queue;
 	unsigned int			n_read;
-	struct tasklet_struct		push;
+	struct work_struct		push;
 
 	struct list_head		write_pool;
 	unsigned int			tx_boundary;
diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
index 3d51e8d82659..c18215751d92 100644
--- a/drivers/usb/host/xhci-dbgtty.c
+++ b/drivers/usb/host/xhci-dbgtty.c
@@ -11,6 +11,7 @@
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
 #include <linux/idr.h>
+#include <linux/workqueue.h>
 
 #include "xhci.h"
 #include "xhci-dbgcap.h"
@@ -189,9 +190,9 @@ dbc_read_complete(struct xhci_dbc *dbc, struct dbc_request *req)
 		}
 	}
 
-	/* defer moving data from req to tty buffer to a tasklet */
+	/* defer moving data from req to tty buffer to BH workqueue */
 	list_add_tail(&req->list_pool, &port->read_queue);
-	tasklet_schedule(&port->push);
+	queue_work(system_bh_wq, &port->push);
 out:
 	spin_unlock_irqrestore(&port->port_lock, flags);
 }
@@ -384,7 +385,7 @@ static void dbc_tty_unthrottle(struct tty_struct *tty)
 	unsigned long		flags;
 
 	spin_lock_irqsave(&port->port_lock, flags);
-	tasklet_schedule(&port->push);
+	queue_work(system_bh_wq, &port->push);
 	spin_unlock_irqrestore(&port->port_lock, flags);
 }
 
@@ -400,13 +401,13 @@ static const struct tty_operations dbc_tty_ops = {
 	.unthrottle		= dbc_tty_unthrottle,
 };
 
-static void dbc_rx_push(struct tasklet_struct *t)
+static void dbc_rx_push(struct work_struct *work)
 {
 	struct dbc_request	*req;
 	struct tty_struct	*tty;
 	unsigned long		flags;
 	bool			disconnect = false;
-	struct dbc_port		*port = from_tasklet(port, t, push);
+	struct dbc_port		*port = from_work(port, work, push);
 	struct list_head	*queue = &port->read_queue;
 	int			untransferred;
 
@@ -438,7 +439,7 @@ static void dbc_rx_push(struct tasklet_struct *t)
 	}
 
 	if (!list_empty(queue))
-		tasklet_schedule(&port->push);
+		queue_work(system_bh_wq, &port->push);
 
 	if (!disconnect)
 		dbc_start_rx(port);
@@ -467,7 +468,7 @@ xhci_dbc_tty_init_port(struct xhci_dbc *dbc, struct dbc_port *port)
 {
 	tty_port_init(&port->port);
 	spin_lock_init(&port->port_lock);
-	tasklet_setup(&port->push, dbc_rx_push);
+	INIT_WORK(&port->push, dbc_rx_push);
 	INIT_LIST_HEAD(&port->read_pool);
 	INIT_LIST_HEAD(&port->read_queue);
 	INIT_LIST_HEAD(&port->write_pool);
@@ -479,7 +480,7 @@ xhci_dbc_tty_init_port(struct xhci_dbc *dbc, struct dbc_port *port)
 static void
 xhci_dbc_tty_exit_port(struct dbc_port *port)
 {
-	tasklet_kill(&port->push);
+	cancel_work_sync(&port->push);
 	tty_port_destroy(&port->port);
 }
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] xhci: dbgtty: convert rx push tasklet to BH workqueue
  2026-10-08 12:06 [PATCH] xhci: dbgtty: convert rx push tasklet to BH workqueue Kun Luo
@ 2026-10-08 12:23 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-08 12:23 UTC (permalink / raw)
  To: Kun Luo; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: xhci-dbgtty: UAF race between teardown and event_work queueing BH
- [High] USB: xhci: DbC TTY double free due to unprotected port->registered check
--

commit c7660e30ebb6c7dc3548c7784608b00e579fc030
Author: Kun Luo <luokun485@gmail.com>

xhci: dbgtty: convert rx push tasklet to BH workqueue
    
The kernel is converting remaining tasklet users to BH workqueues so
softirq handling can go through the workqueue path. The dbc TTY rx
push path is a straightforward bottom-half: schedule from IRQ /
unthrottle, run the same work, kill on port teardown.

> diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
> index 3d51e8d82659d..c18215751d921 100644
> --- a/drivers/usb/host/xhci-dbgtty.c
> +++ b/drivers/usb/host/xhci-dbgtty.c

[ ... ]

> @@ -189,9 +190,9 @@ dbc_read_complete(struct xhci_dbc *dbc, struct dbc_request *req)
>  		}
>  	}
>  
> -	/* defer moving data from req to tty buffer to a tasklet */
> +	/* defer moving data from req to tty buffer to BH workqueue */
>  	list_add_tail(&req->list_pool, &port->read_queue);
> -	tasklet_schedule(&port->push);
> +	queue_work(system_bh_wq, &port->push);

[Severity: High]
Does this code introduce a use-after-free race during teardown?

If a USB hot-unplug or unbind event occurs while the device is in the
DS_CONFIGURED state, and a bulk transfer event is actively being processed
by dbc->event_work on another CPU:

CPU0 (teardown via xhci_dbc_stop):
Calls dbc->driver->disconnect() which calls cancel_work_sync(&port->push).
After disconnect completes, it calls cancel_delayed_work_sync(&dbc->event_work).

CPU1 (concurrent event processing):
event_work calls xhci_dbc_giveback() which drops dbc->lock.
req->complete() calls dbc_read_complete().
queue_work(system_bh_wq, &port->push) is executed.

Because cancel_work_sync() on CPU0 has already finished, the BH work is queued
on a device that is about to be freed. Once CPU0's teardown completes,
xhci_dbc_tty_remove() frees port and dbc. The scheduled BH work subsequently
executes dbc_rx_push(), dereferencing the freed port pointer.

>  out:
>  	spin_unlock_irqrestore(&port->port_lock, flags);
>  }

[ ... ]

> @@ -479,7 +480,7 @@ xhci_dbc_tty_init_port(struct xhci_dbc *dbc, struct dbc_port *port)
>  static void
>  xhci_dbc_tty_exit_port(struct dbc_port *port)
>  {
> -	tasklet_kill(&port->push);
> +	cancel_work_sync(&port->push);

[Severity: High]
Can this destruction path race with concurrent disconnect events, leading to a
double free of TTY and FIFO resources?

If a concurrent sysfs disable write invokes xhci_dbc_stop(), and a physical
cable unplug event is processed by dbc->event_work calling
xhci_dbc_handle_events():

xhci_dbc_stop() reads dbc->state == DS_CONFIGURED under dbc->lock, but drops
the lock without changing the state. This allows event_work to also observe
DS_CONFIGURED, handle the unplug, and return EVT_DISC.

Both threads then concurrently invoke dbc->driver->disconnect().

Since xhci_dbc_tty_unregister_device() checks and clears port->registered
locklessly before calling xhci_dbc_tty_exit_port(), both threads bypass the
check and proceed with destruction. This causes a double execution of
tty_port_destroy(), idr_remove(), and kfifo_free().

>  	tty_port_destroy(&port->port);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008120605.174601-1-luokun485@gmail.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-08 12:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 12:06 [PATCH] xhci: dbgtty: convert rx push tasklet to BH workqueue Kun Luo
2026-10-08 12:23 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox