[PATCH] usb: gadget: udc: amd5536udc_pci fix crash if dma is used

[PATCH] usb: gadget: udc: amd5536udc_pci fix crash if dma is used

[Tong Zhang]

init_dma_pools() calls dma_pool_create(...dev->dev) to create dma pool.
however, dev->dev is actually set after calling init_dma_pools(), which
effectively makes dma_pool_create(..NULL) and cause crash.
To fix this issue, init dma only after dev->dev is set.

[    1.317993] RIP: 0010:dma_pool_create+0x83/0x290
[    1.323257] Call Trace:
[    1.323390]  ? pci_write_config_word+0x27/0x30
[    1.323626]  init_dma_pools+0x41/0x1a0 [snps_udc_core]
[    1.323899]  udc_pci_probe+0x202/0x2b1 [amd5536udc_pci]

Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 drivers/usb/gadget/udc/amd5536udc_pci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
index 8d387e0e4d91..c80f9bd51b75 100644
--- a/drivers/usb/gadget/udc/amd5536udc_pci.c
+++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
@@ -153,6 +153,11 @@ static int udc_pci_probe(
 	pci_set_master(pdev);
 	pci_try_set_mwi(pdev);
 
+	dev->phys_addr = resource;
+	dev->irq = pdev->irq;
+	dev->pdev = pdev;
+	dev->dev = &pdev->dev;
+
 	/* init dma pools */
 	if (use_dma) {
 		retval = init_dma_pools(dev);
@@ -160,11 +165,6 @@ static int udc_pci_probe(
 			goto err_dma;
 	}
 
-	dev->phys_addr = resource;
-	dev->irq = pdev->irq;
-	dev->pdev = pdev;
-	dev->dev = &pdev->dev;
-
 	/* general probing */
 	if (udc_probe(dev)) {
 		retval = -ENODEV;
-- 
2.25.1

Re: [PATCH] usb: gadget: udc: amd5536udc_pci fix crash if dma is used

[Greg Kroah-Hartman]

On Sat, Mar 13, 2021 at 02:20:32PM -0500, Tong Zhang wrote:
> init_dma_pools() calls dma_pool_create(...dev->dev) to create dma pool.
> however, dev->dev is actually set after calling init_dma_pools(), which
> effectively makes dma_pool_create(..NULL) and cause crash.
> To fix this issue, init dma only after dev->dev is set.
> 
> [    1.317993] RIP: 0010:dma_pool_create+0x83/0x290
> [    1.323257] Call Trace:
> [    1.323390]  ? pci_write_config_word+0x27/0x30
> [    1.323626]  init_dma_pools+0x41/0x1a0 [snps_udc_core]
> [    1.323899]  udc_pci_probe+0x202/0x2b1 [amd5536udc_pci]
> 
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>

What commit caused this problem?

Can you resend this with a "Fixes:" tag added showing that?

thanks,

greg k-h

[PATCH v2] usb: gadget: udc: amd5536udc_pci fix null-ptr-dereference

[Tong Zhang]

init_dma_pools() calls dma_pool_create(...dev->dev) to create dma pool.
however, dev->dev is actually set after calling init_dma_pools(), which
effectively makes dma_pool_create(..NULL) and cause crash.
To fix this issue, init dma only after dev->dev is set.

[    1.317993] RIP: 0010:dma_pool_create+0x83/0x290
[    1.323257] Call Trace:
[    1.323390]  ? pci_write_config_word+0x27/0x30
[    1.323626]  init_dma_pools+0x41/0x1a0 [snps_udc_core]
[    1.323899]  udc_pci_probe+0x202/0x2b1 [amd5536udc_pci]

Fixes: 7c51247a1f62 (usb: gadget: udc: Provide correct arguments for 'dma_pool_create')
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
v2: add Fixes tag and revise subject

 drivers/usb/gadget/udc/amd5536udc_pci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/udc/amd5536udc_pci.c b/drivers/usb/gadget/udc/amd5536udc_pci.c
index 8d387e0e4d91..c80f9bd51b75 100644
--- a/drivers/usb/gadget/udc/amd5536udc_pci.c
+++ b/drivers/usb/gadget/udc/amd5536udc_pci.c
@@ -153,6 +153,11 @@ static int udc_pci_probe(
 	pci_set_master(pdev);
 	pci_try_set_mwi(pdev);
 
+	dev->phys_addr = resource;
+	dev->irq = pdev->irq;
+	dev->pdev = pdev;
+	dev->dev = &pdev->dev;
+
 	/* init dma pools */
 	if (use_dma) {
 		retval = init_dma_pools(dev);
@@ -160,11 +165,6 @@ static int udc_pci_probe(
 			goto err_dma;
 	}
 
-	dev->phys_addr = resource;
-	dev->irq = pdev->irq;
-	dev->pdev = pdev;
-	dev->dev = &pdev->dev;
-
 	/* general probing */
 	if (udc_probe(dev)) {
 		retval = -ENODEV;
-- 
2.25.1

Re: [PATCH] usb: gadget: udc: amd5536udc_pci fix crash if dma is used

[Tong Zhang]

Hi Greg,
Thanks for the comments.
I tried to find the commit that introduced the use of dev->dev in
init_dma_pools()
and added that commit to the Fixes tag in v2 patch.
Please see if this one works.
Thanks,
- Tong