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

[Rob Herring <robh@kernel.org>]

On Fri, Aug 13, 2021 at 7:55 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-08-13 12:33, Dan Carpenter wrote:
> > 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.

>
> Robin.
>
> > Fixes: e1229e884e19 ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >   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) {
> >