Re: [PATCH] PCI: rockchip-dwc: Potential error pointer dereference in probe

On 2021-08-13 14:34, Rob Herring wrote:

On Fri, Aug 13, 2021 at 7:55 AM Robin Murphy [off-list ref] wrote:

quoted

On 2021-08-13 12:33, Dan Carpenter wrote:

quoted

If devm_regulator_get_optional() returns an error pointer, then we
should return it to the user.  The current code makes an exception
for -ENODEV that will result in an error pointer dereference on the
next line when it calls regulator_enable().  Remove the exception.

Doesn't this break the apparent intent of the regulator being optional,
though?

I'm pretty sure 'optional' means ENODEV is never returned. So there
wasn't any real problem, but the check was unnecessary.

In fact it's the other way round - "optional" in this case is for when 
the supply may legitimately not exist so the driver may or may not need 
to handle it, so it can return -ENODEV if a regulator isn't described by 
firmware. A non-optional regulator is assumed to represent a necessary 
supply, so if there's nothing described by firmware you get the (valid) 
dummy regulator back.

Robin.

quoted

quoted

Fixes: e1229e884e19 ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")
Signed-off-by: Dan Carpenter <redacted>
---
   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 5 ++---
   1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 20cef2e06f66..2d0ffd3c4e16 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c

@@ -225,9 +225,8 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
       /* DON'T MOVE ME: must be enable before PHY init */
       rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
       if (IS_ERR(rockchip->vpcie3v3))
-             if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV)
-                     return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
-                                     "failed to get vpcie3v3 regulator\n");
+             return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
+                                  "failed to get vpcie3v3 regulator\n");

       ret = regulator_enable(rockchip->vpcie3v3);
       if (ret) {

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel