Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ricardo Ribalda Delgado <ribalda@chromium.org>
Date:   Thu, 01 Jun 2023 15:51:19 +0200
Subject: [PATCH v2] xhci: Do not create endpoint debugfs while holding the
 bandwidth mutex
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-Id: <20230531-xhci-deadlock-v2-1-5f5cf8ff5285@chromium.org>
To:     Mathias Nyman <mathias.nyman@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Mathias Nyman <mathias.nyman@linux.intel.com>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        Stephen Boyd <swboyd@chromium.org>,
        Ricardo Ribalda Delgado <ribalda@chromium.org>
Precedence: bulk

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@chromium.org>
Fixes: 167657a1bb5f ("xhci: don't create endpoint debugfs entry before ring buffer is set.")
Signed-off-by: Ricardo Ribalda Delgado <ribalda@chromium.org>
---
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@chromium.org
---
 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@chromium.org>