[PATCH] xhci: fix lost bounce buffers on TDs spanning several ring segments
Arthur Gautier <[email protected]>
| Newsgroups | org.kernel.vger.linux-usb,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
When a TD reaches a link TRB with data that is not aligned to the
endpoint's wMaxPacketSize, xhci_align_td() stages the unalignable tail
through the bounce buffer of the ring segment holding that link TRB.
xhci_unmap_td_bounce_buffer() later unmaps it and, for IN transfers,
copies the data back into the URB's buffer.
The enqueue path records the segment that was bounced in td->bounce_seg,
under the assumption that a TD never spans more than two ring segments.
That assumption does not hold: a TD large enough to span three or more
segments crosses several link TRBs and can be bounced at each of them.
Only the last one survives in td->bounce_seg, so every earlier bounce
buffer is neither copied back nor DMA unmapped.
The URB still completes with actual_length equal to the requested length
and no error, so the transfer looks successful while a wMaxPacketSize
sized hole in the destination buffer silently keeps its previous
contents. It also leaks a DMA mapping per dropped bounce.
This is reachable with a USB mass storage device behind xHCI backing a
dm-verity target with 512 byte hash blocks. The device enumerates as
SuperSpeed, so wMaxPacketSize is 1024, while dm-bufio issues one 512 byte
bio per hash block. verity_prefetch_io() makes the block layer merge
hundreds of them into a single request of up to 512 scatterlist entries
of 512 bytes each. At 256 TRBs per ring segment such a TD spans three
segments, and every segment boundary falls on an odd multiple of 512,
i.e. unaligned to wMaxPacketSize. dm-bufio then caches a hash block
holding stale data and dm-verity declares the metadata block corrupted:
device-mapper: verity: 8:2: metadata block 10850 is corrupted
A reproducer running this under qemu is available at
https://github.com/baloo/xhci-verity
The bounce state (bounce_buf, bounce_dma, bounce_len, bounce_offs)
already lives on the ring segment, so there is nothing extra to track.
Keep the first bounced segment in td->bounce_seg and, on completion,
walk the segments the TD covers from there up to td->end_seg, handling
every segment that still has a pending bounce.
Fixes: f9c589e142d0 ("xhci: TD-fragment, align the unsplittable case with a bounce buffer")
Cc: Mathias Nyman <[email protected]>
Cc: [email protected]
Signed-off-by: Arthur Gautier <[email protected]>
---
drivers/usb/host/xhci-ring.c | 49 +++++++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 4f98d8269625..4154e84420ac 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -842,21 +842,18 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci,
usb_hcd_giveback_urb(hcd, urb, status);
}
-static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
- struct xhci_ring *ring, struct xhci_td *td)
+static void xhci_unmap_one_bounce_buffer(struct xhci_hcd *xhci,
+ struct xhci_ring *ring, struct xhci_td *td,
+ struct xhci_segment *seg)
{
struct device *dev = xhci_to_hcd(xhci)->self.sysdev;
- struct xhci_segment *seg = td->bounce_seg;
struct urb *urb = td->urb;
size_t len;
- if (!ring || !seg || !urb)
- return;
-
if (usb_urb_dir_out(urb)) {
dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
DMA_TO_DEVICE);
- return;
+ goto done;
}
dma_unmap_single(dev, seg->bounce_dma, ring->bounce_buf_len,
@@ -872,10 +869,37 @@ static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
memcpy(urb->transfer_buffer + seg->bounce_offs, seg->bounce_buf,
seg->bounce_len);
}
+done:
seg->bounce_len = 0;
seg->bounce_offs = 0;
}
+static void xhci_unmap_td_bounce_buffer(struct xhci_hcd *xhci,
+ struct xhci_ring *ring, struct xhci_td *td)
+{
+ struct xhci_segment *seg;
+ unsigned int i;
+
+ if (!ring || !td->bounce_seg || !td->urb)
+ return;
+
+ /*
+ * A TD that spans more than two ring segments crosses several link
+ * TRBs, and may have been aligned with a bounce buffer at each of
+ * them. Every bounce buffer lives on the segment whose link TRB it
+ * was needed for, so walk all segments the TD covers, starting at
+ * the first one that was bounced.
+ */
+ seg = td->bounce_seg;
+ for (i = 0; i < ring->num_segs; i++) {
+ if (seg->bounce_len)
+ xhci_unmap_one_bounce_buffer(xhci, ring, td, seg);
+ if (seg == td->end_seg)
+ break;
+ seg = seg->next;
+ }
+}
+
static void xhci_td_cleanup(struct xhci_hcd *xhci, struct xhci_td *td,
struct xhci_ring *ep_ring, int status)
{
@@ -3674,8 +3698,15 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
&trb_buff_len,
ring->enq_seg)) {
send_addr = ring->enq_seg->bounce_dma;
- /* assuming TD won't span 2 segs */
- td->bounce_seg = ring->enq_seg;
+ /*
+ * A TD spanning several segments can be
+ * bounced once per segment boundary it
+ * crosses. Remember the first bounced
+ * segment, the rest are found by walking
+ * the TD's segments on completion.
+ */
+ if (!td->bounce_seg)
+ td->bounce_seg = ring->enq_seg;
}
}
}
--
2.55.0