Re: possible recursive locking detected in kernel v5.18

[Alan Stern <stern@rowland.harvard.edu>]

On Fri, Aug 12, 2022 at 10:56:08AM +0800, Rondreis wrote:
> Hi, after adding the patch, this bug seems unfixed, with the following
> console output: https://pastebin.com/pUUitSJ8
> 
> Alan Stern <stern@rowland.harvard.edu> 于2022年8月12日周五 00:58写道：
> >
> > On Thu, Aug 11, 2022 at 10:06:37AM +0800, Rondreis wrote:
> > > Hello,
> > >
> > > When fuzzing the Linux kernel driver v5.18.0, the following crash was triggered.

> > > It seems that there is a deadlock happened in function usb_stor_post_reset
> > >
> > > The crash report is as follow:
> > >
> > > ```
> > > usb 7-1: r8712u: Loading firmware from "rtlwifi/rtl8712u.bin"
> > > ============================================
> > > WARNING: possible recursive locking detected
> > > 5.18.0 #3 Not tainted
> > > --------------------------------------------
> > > kworker/1:3/1205 is trying to acquire lock:
> > > ffff888018638db8 (&us_interface_key[i]){+.+.}-{3:3}, at:
> > > usb_stor_pre_reset+0x35/0x40 drivers/usb/storage/usb.c:230
> > >
> > > but task is already holding lock:
> > > ffff888018638db8 (&us_interface_key[i]){+.+.}-{3:3}, at:
> > > usb_stor_pre_reset+0x35/0x40 drivers/usb/storage/usb.c:230

Rondreis, can you please try testing the patch below instead of the one 
I sent you last week?

Alan Stern



Index: usb-devel/drivers/usb/core/hub.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hub.c
+++ usb-devel/drivers/usb/core/hub.c
@@ -6048,6 +6048,11 @@ re_enumerate:
  * the reset is over (using their post_reset method).
  *
  * Return: The same as for usb_reset_and_verify_device().
+ * However, if a reset is already in progress (for instance, if a
+ * driver doesn't have pre_ or post_reset() callbacks, and while
+ * being unbound or re-bound during the ongoing reset its disconnect()
+ * or probe() routine tries to perform a second, nested reset), the
+ * routine returns -EINPROGRESS.
  *
  * Note:
  * The caller must own the device lock.  For example, it's safe to use
@@ -6081,6 +6086,10 @@ int usb_reset_device(struct usb_device *
 		return -EISDIR;
 	}
 
+	if (udev->reset_in_progress)
+		return -EINPROGRESS;
+	udev->reset_in_progress = 1;
+
 	port_dev = hub->ports[udev->portnum - 1];
 
 	/*
@@ -6145,6 +6154,7 @@ int usb_reset_device(struct usb_device *
 
 	usb_autosuspend_device(udev);
 	memalloc_noio_restore(noio_flag);
+	udev->reset_in_progress = 0;
 	return ret;
 }
 EXPORT_SYMBOL_GPL(usb_reset_device);
Index: usb-devel/include/linux/usb.h
===================================================================
--- usb-devel.orig/include/linux/usb.h
+++ usb-devel/include/linux/usb.h
@@ -575,6 +575,7 @@ struct usb3_lpm_parameters {
  * @devaddr: device address, XHCI: assigned by HW, others: same as devnum
  * @can_submit: URBs may be submitted
  * @persist_enabled:  USB_PERSIST enabled for this device
+ * @reset_in_progress: the device is being reset
  * @have_langid: whether string_langid is valid
  * @authorized: policy has said we can use it;
  *	(user space) policy determines if we authorize this device to be
@@ -661,6 +662,7 @@ struct usb_device {
 
 	unsigned can_submit:1;
 	unsigned persist_enabled:1;
+	unsigned reset_in_progress:1;
 	unsigned have_langid:1;
 	unsigned authorized:1;
 	unsigned authenticated:1;