Skip to content

Commit aa55e09

Browse files
matnymangregkh
authored andcommitted
xhci: dbgtty: Fix data corruption when transmitting data form DbC to host
commit f6bb3b6 upstream. Data read from a DbC device may be corrupted due to a race between ongoing write and write request completion handler both queuing new transfer blocks (TRBs) if there are remining data in the kfifo. TRBs may be in incorrct order compared to the data in the kfifo. Driver fails to keep lock between reading data from kfifo into a dbc request buffer, and queuing the request to the transfer ring. This allows completed request to re-queue itself in the middle of an ongoing transfer loop, forcing itself between a kfifo read and request TRB write of another request cpu0 cpu1 (re-queue completed req2) lock(port_lock) dbc_start_tx() kfifo_out(fifo, req1->buffer) unlock(port_lock) lock(port_lock) dbc_write_complete(req2) dbc_start_tx() kfifo_out(fifo, req2->buffer) unlock(port_lock) lock(port_lock) req2->trb = ring->enqueue; ring->enqueue++ unlock(port_lock) lock(port_lock) req1->trb = ring->enqueue; ring->enqueue++ unlock(port_lock) In the above scenario a kfifo containing "12345678" would read "1234" to req1 and "5678" to req2, but req2 is queued before req1 leading to data being transmitted as "56781234" Solve this by adding a flag that prevents starting a new tx if we are already mid dbc_start_tx() during the unlocked part. The already running dbc_do_start_tx() will make sure the newly completed request gets re-queued as it is added to the request write_pool while holding the lock. Cc: stable@vger.kernel.org Fixes: dfba217 ("usb: xhci: Add DbC support in xHCI driver") Tested-by: Łukasz Bartosik <ukaszb@chromium.org> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Link: https://patch.msgid.link/20251107162819.1362579-3-mathias.nyman@linux.intel.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d77efd7 commit aa55e09

File tree

2 files changed

+17
-1
lines changed

2 files changed

+17
-1
lines changed

drivers/usb/host/xhci-dbgcap.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ struct dbc_port {
114114
unsigned int tx_boundary;
115115

116116
bool registered;
117+
bool tx_running;
117118
};
118119

119120
struct dbc_driver {

drivers/usb/host/xhci-dbgtty.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ dbc_kfifo_to_req(struct dbc_port *port, char *packet)
4747
return len;
4848
}
4949

50-
static int dbc_start_tx(struct dbc_port *port)
50+
static int dbc_do_start_tx(struct dbc_port *port)
5151
__releases(&port->port_lock)
5252
__acquires(&port->port_lock)
5353
{
@@ -57,6 +57,8 @@ static int dbc_start_tx(struct dbc_port *port)
5757
bool do_tty_wake = false;
5858
struct list_head *pool = &port->write_pool;
5959

60+
port->tx_running = true;
61+
6062
while (!list_empty(pool)) {
6163
req = list_entry(pool->next, struct dbc_request, list_pool);
6264
len = dbc_kfifo_to_req(port, req->buf);
@@ -77,12 +79,25 @@ static int dbc_start_tx(struct dbc_port *port)
7779
}
7880
}
7981

82+
port->tx_running = false;
83+
8084
if (do_tty_wake && port->port.tty)
8185
tty_wakeup(port->port.tty);
8286

8387
return status;
8488
}
8589

90+
/* must be called with port->port_lock held */
91+
static int dbc_start_tx(struct dbc_port *port)
92+
{
93+
lockdep_assert_held(&port->port_lock);
94+
95+
if (port->tx_running)
96+
return -EBUSY;
97+
98+
return dbc_do_start_tx(port);
99+
}
100+
86101
static void dbc_start_rx(struct dbc_port *port)
87102
__releases(&port->port_lock)
88103
__acquires(&port->port_lock)

0 commit comments

Comments
 (0)