[PATCH] usb: gadget: pch_udc: don't call pch_udc_remove() from the probe error path

[PATCH] usb: gadget: pch_udc: don't call pch_udc_remove() from the probe error path

[Vasileios Almpanis]

Probe error paths go to finished label which calls phc_udc_remove. It in
turn starts with an unconditional usb_del_gadget_udc(). If probe fails
before the gadget is added then we put a non-initialized kobject
(gadget.dev) and a WARN is triggered in kobject_put().

  WARNING: lib/kobject.c:734 at kobject_put+0x252/0x640 lib/kobject.c:734
  Call Trace:
   put_device+0x1f/0x30 drivers/base/core.c:3880
   pch_udc_remove+0x47/0x510 drivers/usb/gadget/udc/pch_udc.c:2986
   pch_udc_probe+0xcc6/0x1130 drivers/usb/gadget/udc/pch_udc.c:3109

If usb_add_gadget_udc() in turn fails, it has already dropped the gadget
reference, so the extra usb_del_gadget_udc() is a double put.

Split the hardware/DMA teardown out into pch_udc_cleanup() and call
only that from the probe error path.

Fixes: 0f91349b89f3 ("usb: gadget: convert all users to the new udc infrastructure")
Reported-by: syzbot+3e2e533aa1b2a75525e7@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=3e2e533aa1b2a75525e7
Tested-by: syzbot+3e2e533aa1b2a75525e7@syzkaller.appspotmail.com
Signed-off-by: Vasileios Almpanis <vasilisalmpanis@gmail.com>
---
 drivers/usb/gadget/udc/pch_udc.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index 0a6886428739..5704aff2b09b 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -2979,16 +2979,8 @@ static void pch_udc_shutdown(struct pci_dev *pdev)
 	pch_udc_set_disconnect(dev);
 }
 
-static void pch_udc_remove(struct pci_dev *pdev)
+static void pch_udc_cleanup(struct pch_udc_dev *dev)
 {
-	struct pch_udc_dev	*dev = pci_get_drvdata(pdev);
-
-	usb_del_gadget_udc(&dev->gadget);
-
-	/* gadget driver must not be registered */
-	if (dev->driver)
-		dev_err(&pdev->dev,
-			"%s: gadget driver still bound!!!\n", __func__);
 	/* dma pool cleanup */
 	dma_pool_destroy(dev->data_requests);
 
@@ -3016,6 +3008,20 @@ static void pch_udc_remove(struct pci_dev *pdev)
 	pch_udc_exit(dev);
 }
 
+static void pch_udc_remove(struct pci_dev *pdev)
+{
+	struct pch_udc_dev	*dev = pci_get_drvdata(pdev);
+
+	usb_del_gadget_udc(&dev->gadget);
+
+	/* gadget driver must not be registered */
+	if (dev->driver)
+		dev_err(&pdev->dev,
+			"%s: gadget driver still bound!!!\n", __func__);
+
+	pch_udc_cleanup(dev);
+}
+
 static int __maybe_unused pch_udc_suspend(struct device *d)
 {
 	struct pch_udc_dev *dev = dev_get_drvdata(d);
@@ -3106,7 +3112,7 @@ static int pch_udc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return 0;
 
 finished:
-	pch_udc_remove(pdev);
+	pch_udc_cleanup(dev);
 	return retval;
 }
 

---
base-commit: d58772d8520c7ef247c4b95c9bd76d3a25da9ff5
change-id: 20260811-pch-6c48d3a7cbe5

Best regards,
--  
Vasileios Almpanis <vasilisalmpanis@gmail.com>

Re: [PATCH] usb: gadget: pch_udc: don't call pch_udc_remove() from the probe error path

[Vasileios Almpanis]

> Probe error paths go to finished label which calls phc_udc_remove. It in
> turn starts with an unconditional usb_del_gadget_udc(). If probe fails
> before the gadget is added then we put a non-initialized kobject
> (gadget.dev) and a WARN is triggered in kobject_put().
> 
>   WARNING: lib/kobject.c:734 at kobject_put+0x252/0x640 lib/kobject.c:734
>   Call Trace:
>    put_device+0x1f/0x30 drivers/base/core.c:3880
>    pch_udc_remove+0x47/0x510 drivers/usb/gadget/udc/pch_udc.c:2986
>    pch_udc_probe+0xcc6/0x1130 drivers/usb/gadget/udc/pch_udc.c:3109
> 
> If usb_add_gadget_udc() in turn fails, it has already dropped the gadget
> reference, so the extra usb_del_gadget_udc() is a double put.
> 
> Split the hardware/DMA teardown out into pch_udc_cleanup() and call
> only that from the probe error path.
> 
Hi everyone!

A gentle ping on this patch. It's already tested by syzbot so it
should be good to go. Please let me know if there is something
I could do differently.

Kind regards,

-- 
Vasileios Almpanis <vasilisalmpanis@gmail.com>