xhci_debugfs_create_endpoint needs to take the mm->mmap_sem, which is
not serialized with the hcd->bandwidth_mutex across the codebase.
Without this patch a deadlock has been observed with the uvc driver at
the functions v4l2_mmap() and usb_set_interface().
Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
Fixes: 167657a1bb5f ("xhci: don't create endpoint debugfs entry before ring buffer is set.")
Signed-off-by: Ricardo Ribalda Delgado <ribalda@xxxxxxxxxxxx>
---
Even though I do not have a proper reproducer for this, I do not see
this deadlock happening after fuzzing locally my kernel.
I am not used to this subsystem, so please take a careful look at
this patch :).
Thanks!
---
Changes in v2:
- Fix Stepeh email. Thanks Greg!
- Update cover to note that with this patch the deadlock does not
happen.
- Link to v1: https://lore.kernel.org/r/20230531-xhci-deadlock-v1-1-57780bff5124@xxxxxxxxxxxx
---
drivers/usb/host/xhci-debugfs.c | 4 ++++
drivers/usb/host/xhci.c | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci-debugfs.c b/drivers/usb/host/xhci-debugfs.c
index 99baa60ef50f..2acce2af2ca9 100644
--- a/drivers/usb/host/xhci-debugfs.c
+++ b/drivers/usb/host/xhci-debugfs.c
@@ -238,6 +238,10 @@ static int xhci_ring_open(struct inode *inode, struct file *file)
int i;
struct xhci_file_map *f_map;
const char *file_name = file_dentry(file)->d_iname;
+ struct xhci_ring *ring = *(struct xhci_ring **)inode->i_private;
+
+ if (!ring)
+ return -EAGAIN;
for (i = 0; i < ARRAY_SIZE(ring_files); i++) {
f_map = &ring_files[i];
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 78790dc13c5f..2715900b2540 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1930,6 +1930,8 @@ int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->in_ctx, ep_index);
trace_xhci_add_endpoint(ep_ctx);
+ xhci_debugfs_create_endpoint(xhci, virt_dev, ep_index);
+
xhci_dbg(xhci, "add ep 0x%x, slot id %d, new drop flags = %#x, new add flags = %#x\n",
(unsigned int) ep->desc.bEndpointAddress,
udev->slot_id,
@@ -2963,7 +2965,6 @@ int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
xhci_check_bw_drop_ep_streams(xhci, virt_dev, i);
virt_dev->eps[i].ring = virt_dev->eps[i].new_ring;
virt_dev->eps[i].new_ring = NULL;
- xhci_debugfs_create_endpoint(xhci, virt_dev, i);
}
command_cleanup:
kfree(command->completion);
@@ -2989,7 +2990,6 @@ void xhci_reset_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
/* Free any rings allocated for added endpoints */
for (i = 0; i < 31; i++) {
if (virt_dev->eps[i].new_ring) {
- xhci_debugfs_remove_endpoint(xhci, virt_dev, i);
xhci_ring_free(xhci, virt_dev->eps[i].new_ring);
virt_dev->eps[i].new_ring = NULL;
}
---
base-commit: 48b1320a674e1ff5de2fad8606bee38f724594dc
change-id: 20230531-xhci-deadlock-de2ab21c90bc
Best regards,
--
Ricardo Ribalda Delgado <ribalda@xxxxxxxxxxxx>
