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

[Robin Murphy]

On 2021-08-13 14:34, Rob Herring wrote:
> On Fri, Aug 13, 2021 at 7:55 AM Robin Murphy <robin.murphy at 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.

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.

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