Re: [PATCH] Revert "PCI: j721e: Drop redundant struct device *"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am Do., 27. Jan. 2022 um 23:29 Uhr schrieb Bjorn Helgaas <helgaas@xxxxxxxxxx>:
>
> On Mon, Jan 24, 2022 at 01:21:22PM +0100, Christian Gmeiner wrote:
> > This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
> >
> > Fixes the following oops:
> >  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
>
> Hi Christian,
>
> Would you mind trying this patch?
>

Works - thanks. You can add my Tested-by.

> commit 9d36a93af8fe ("PCI: j721e: Initialize pcie->cdns_pcie before using it")
> Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Date:   Thu Jan 27 15:49:49 2022 -0600
>
>     PCI: j721e: Initialize pcie->cdns_pcie before using it
>
>     Christian reported a NULL pointer dereference in j721e_pcie_probe() caused
>     by 19e863828acf ("PCI: j721e: Drop redundant struct device *"), which
>     removed struct j721e_pcie.dev since there's another copy in struct
>     cdns_pcie.dev reachable via j721e_pcie->cdns_pcie->dev.
>
>     The problem is that j721e_pcie->cdns_pcie was dereferenced before being
>     initialized:
>
>       j721e_pcie_probe
>         pcie = devm_kzalloc()             # struct j721e_pcie
>         j721e_pcie_ctrl_init(pcie)
>           dev = pcie->cdns_pcie->dev      <-- dereference cdns_pcie
>         switch (mode) {
>         case PCI_MODE_RC:
>           cdns_pcie = ...                 # alloc as part of pci_host_bridge
>           pcie->cdns_pcie = cdns_pcie     <-- initialize pcie->cdns_pcie
>
>     Initialize pcie->cdns_pcie before it is used.
>
>     Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
>     Reported-by: Christian Gmeiner <christian.gmeiner@xxxxxxxxx>
>     Link: https://lore.kernel.org/r/20220124122132.435743-1-christian.gmeiner@xxxxxxxxx
>     Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 489586a4cdc7..5d950c1d9fd0 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -372,10 +372,48 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
>         mode = (u32)data->mode;
>
> +       switch (mode) {
> +       case PCI_MODE_RC:
> +               if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST))
> +                       return -ENODEV;
> +
> +               bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> +               if (!bridge)
> +                       return -ENOMEM;
> +
> +               if (!data->byte_access_allowed)
> +                       bridge->ops = &cdns_ti_pcie_host_ops;
> +               rc = pci_host_bridge_priv(bridge);
> +               rc->quirk_retrain_flag = data->quirk_retrain_flag;
> +               rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> +
> +               cdns_pcie = &rc->pcie;
> +               break;
> +       case PCI_MODE_EP:
> +               if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP))
> +                       return -ENODEV;
> +
> +               ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> +               if (!ep)
> +                       return -ENOMEM;
> +
> +               ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> +
> +               cdns_pcie = &ep->pcie;
> +               break;
> +       default:
> +               dev_err(dev, "INVALID device type %d\n", mode);
> +               return 0;
> +       }
> +
> +       cdns_pcie->dev = dev;
> +       cdns_pcie->ops = &j721e_pcie_ops;
> +
>         pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
>         if (!pcie)
>                 return -ENOMEM;
>
> +       pcie->cdns_pcie = cdns_pcie;
>         pcie->mode = mode;
>         pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
>
> @@ -426,28 +464,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
>         switch (mode) {
>         case PCI_MODE_RC:
> -               if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) {
> -                       ret = -ENODEV;
> -                       goto err_get_sync;
> -               }
> -
> -               bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> -               if (!bridge) {
> -                       ret = -ENOMEM;
> -                       goto err_get_sync;
> -               }
> -
> -               if (!data->byte_access_allowed)
> -                       bridge->ops = &cdns_ti_pcie_host_ops;
> -               rc = pci_host_bridge_priv(bridge);
> -               rc->quirk_retrain_flag = data->quirk_retrain_flag;
> -               rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> -
> -               cdns_pcie = &rc->pcie;
> -               cdns_pcie->dev = dev;
> -               cdns_pcie->ops = &j721e_pcie_ops;
> -               pcie->cdns_pcie = cdns_pcie;
> -
>                 gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>                 if (IS_ERR(gpiod)) {
>                         ret = PTR_ERR(gpiod);
> @@ -497,23 +513,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
>                 break;
>         case PCI_MODE_EP:
> -               if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP)) {
> -                       ret = -ENODEV;
> -                       goto err_get_sync;
> -               }
> -
> -               ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> -               if (!ep) {
> -                       ret = -ENOMEM;
> -                       goto err_get_sync;
> -               }
> -               ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> -
> -               cdns_pcie = &ep->pcie;
> -               cdns_pcie->dev = dev;
> -               cdns_pcie->ops = &j721e_pcie_ops;
> -               pcie->cdns_pcie = cdns_pcie;
> -
>                 ret = cdns_pcie_init_phy(dev, cdns_pcie);
>                 if (ret) {
>                         dev_err(dev, "Failed to init phy\n");
> @@ -525,8 +524,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>                         goto err_pcie_setup;
>
>                 break;
> -       default:
> -               dev_err(dev, "INVALID device type %d\n", mode);
>         }
>
>         return 0;



-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux