| Message ID | 20241121094020.3679787-1-wenst@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | PCI/pwrctl: Do not assume device node presence | expand |
On Thu, Nov 21, 2024 at 10:40 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > A PCI device normally does not have a device node, since the bus is > fully enumerable. Assuming that a device node is presence is likely s/presence/present/ > bad. > > The newly added pwrctl code assumes such and crashes with a NULL > pointer dereference. Besides that, of_find_device_by_node(NULL) > is likely going to return some random device. > > Reported-by: Klara Modin <klarasmodin@gmail.com> > Closes: https://lore.kernel.org/linux-pci/a7b8f84d-efa6-490c-8594-84c1de9a7031@gmail.com/ > Fixes: cc70852b0962 ("PCI/pwrctl: Ensure that pwrctl drivers are probed before PCI client drivers") > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Cc: Krzysztof Wilczyński <kwilczynski@kernel.org> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Cc: stable+noautosel@kernel.org # Depends on power supply check > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > drivers/pci/bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 98910bc0fcc4..eca72e0c3b6c 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -405,7 +405,7 @@ void pci_bus_add_device(struct pci_dev *dev) > * before PCI client drivers. > */ > pdev = of_find_device_by_node(dn); > - if (pdev && of_pci_supply_present(dn)) { > + if (dn && pdev && of_pci_supply_present(dn)) { > if (!device_link_add(&dev->dev, &pdev->dev, > DL_FLAG_AUTOREMOVE_CONSUMER)) > pci_err(dev, "failed to add device link to power control device %s\n", > -- > 2.47.0.338.g60cca15819-goog > > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On 2024-11-21 10:40, Chen-Yu Tsai wrote: > A PCI device normally does not have a device node, since the bus is > fully enumerable. Assuming that a device node is presence is likely > bad. > > The newly added pwrctl code assumes such and crashes with a NULL > pointer dereference. Besides that, of_find_device_by_node(NULL) > is likely going to return some random device. > > Reported-by: Klara Modin <klarasmodin@gmail.com> > Closes: https://lore.kernel.org/linux-pci/a7b8f84d-efa6-490c-8594-84c1de9a7031@gmail.com/ > Fixes: cc70852b0962 ("PCI/pwrctl: Ensure that pwrctl drivers are probed before PCI client drivers") > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Cc: Krzysztof Wilczyński <kwilczynski@kernel.org> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Cc: stable+noautosel@kernel.org # Depends on power supply check > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > drivers/pci/bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 98910bc0fcc4..eca72e0c3b6c 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -405,7 +405,7 @@ void pci_bus_add_device(struct pci_dev *dev) > * before PCI client drivers. > */ > pdev = of_find_device_by_node(dn); > - if (pdev && of_pci_supply_present(dn)) { > + if (dn && pdev && of_pci_supply_present(dn)) { > if (!device_link_add(&dev->dev, &pdev->dev, > DL_FLAG_AUTOREMOVE_CONSUMER)) > pci_err(dev, "failed to add device link to power control device %s\n", Thanks for the fix, Tested-by: Klara Modin <klarasmodin@gmail.com>
On Thu, Nov 21, 2024 at 05:40:19PM +0800, Chen-Yu Tsai wrote: > A PCI device normally does not have a device node, since the bus is > fully enumerable. Assuming that a device node is presence is likely > bad. > I missed the fact that NULL ptr check is removed from of_pci_supply_present(). > The newly added pwrctl code assumes such and crashes with a NULL > pointer dereference. Besides that, of_find_device_by_node(NULL) > is likely going to return some random device. > Yeah, good catch. > Reported-by: Klara Modin <klarasmodin@gmail.com> > Closes: https://lore.kernel.org/linux-pci/a7b8f84d-efa6-490c-8594-84c1de9a7031@gmail.com/ > Fixes: cc70852b0962 ("PCI/pwrctl: Ensure that pwrctl drivers are probed before PCI client drivers") > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Cc: Krzysztof Wilczyński <kwilczynski@kernel.org> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > Cc: stable+noautosel@kernel.org # Depends on power supply check > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> Thanks for the fix! Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/pci/bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 98910bc0fcc4..eca72e0c3b6c 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -405,7 +405,7 @@ void pci_bus_add_device(struct pci_dev *dev) > * before PCI client drivers. > */ > pdev = of_find_device_by_node(dn); > - if (pdev && of_pci_supply_present(dn)) { > + if (dn && pdev && of_pci_supply_present(dn)) { > if (!device_link_add(&dev->dev, &pdev->dev, > DL_FLAG_AUTOREMOVE_CONSUMER)) > pci_err(dev, "failed to add device link to power control device %s\n", > -- > 2.47.0.338.g60cca15819-goog >
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 98910bc0fcc4..eca72e0c3b6c 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -405,7 +405,7 @@ void pci_bus_add_device(struct pci_dev *dev) * before PCI client drivers. */ pdev = of_find_device_by_node(dn); - if (pdev && of_pci_supply_present(dn)) { + if (dn && pdev && of_pci_supply_present(dn)) { if (!device_link_add(&dev->dev, &pdev->dev, DL_FLAG_AUTOREMOVE_CONSUMER)) pci_err(dev, "failed to add device link to power control device %s\n",
A PCI device normally does not have a device node, since the bus is fully enumerable. Assuming that a device node is presence is likely bad. The newly added pwrctl code assumes such and crashes with a NULL pointer dereference. Besides that, of_find_device_by_node(NULL) is likely going to return some random device. Reported-by: Klara Modin <klarasmodin@gmail.com> Closes: https://lore.kernel.org/linux-pci/a7b8f84d-efa6-490c-8594-84c1de9a7031@gmail.com/ Fixes: cc70852b0962 ("PCI/pwrctl: Ensure that pwrctl drivers are probed before PCI client drivers") Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Cc: Krzysztof Wilczyński <kwilczynski@kernel.org> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> Cc: stable+noautosel@kernel.org # Depends on power supply check Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/pci/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)