Re: [PATCH] usb: cdns3: Fix NULL pointer dereference in cdns3_pci_probe

On 26-08-06 10:22:58, Jie Deng wrote:
> The Cadence USBSS controller is a two-function PCI device. The first
> probed function allocates the driver data and stores it with
> pci_set_drvdata(), while the second function reuses it via
> pci_get_drvdata() when pci_is_enabled() reports that the first
> function has already been probed.
> 
> When the second function is probed while the first one has been
> enabled but has not yet set its driver data, pci_get_drvdata()
> returns NULL, and the subsequent wrap->devfn assignment dereferences
> a NULL pointer and crashes the kernel.
> logs:
> Call trace:
>   cdns3_pci_probe+0xa4/0x300
>   local_pci_probe+0x44/0xa8
>   pci_call_probe+0x54/0x158
>   pci_device_probe+0x84/0x100
>   really_probe+0x184/0x3d0
>   __driver_probe_device+0x80/0x178
>   driver_probe_device+0x44/0xe8
>   __driver_attach+0xec/0x1f8
>   bus_for_each_dev+0x7c/0xe0
>   driver_attach+0x28/0x38
>   bus_add_driver+0x110/0x238
>   driver_register+0x64/0x128
>   __pci_register_driver+0x50/0x60
>   cdns3_pci_driver_init+0x28/0x38
>   do_one_initcall+0x5c/0x280
>   do_initcalls+0x104/0x1d8
>   kernel_init_freeable+0x140/0x218
>   kernel_init+0x28/0x1f8
>   ret_from_fork+0x10/0x20
> 
> Return -EPROBE_DEFER in this case so that probing is retried after
> the first function has completed its probe.
> 
> Fixes: 8bc1901ca7b0 ("usb:cdns3 Add Cadence USB3 DRD Driver")

The patch is okay, but the Fixes tag is wrong, 8bc1901ca7b0 was reverted,
you may use below tag:

Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")

After above fix, feel free add:
Acked-by: Peter Chen <[email protected]>

Peter

> Signed-off-by: Jie Deng <[email protected]>
> ---
>  drivers/usb/cdns3/cdns3-pci-wrap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
> index eb5760f75b9d..fd06a3d8e638 100644
> --- a/drivers/usb/cdns3/cdns3-pci-wrap.c
> +++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
> @@ -96,6 +96,11 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  
>  	if (pci_is_enabled(func)) {
>  		wrap = pci_get_drvdata(func);
> +		if (!wrap) {
> +			dev_err(&pdev->dev,
> +				"second function not initialized, retrying\n");
> +			return -EPROBE_DEFER;
> +		}
>  	} else {
>  		wrap = kzalloc_obj(*wrap);
>  		if (!wrap)
> -- 
> 2.25.1
> 

-- 

Thanks,
Peter Chen