* [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe()
@ 2020-04-27 11:10 Wei Yongjun
2020-04-27 11:59 ` Vignesh Raghavendra
2020-04-29 1:50 ` [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname() Wei Yongjun
0 siblings, 2 replies; 7+ messages in thread
From: Wei Yongjun @ 2020-04-27 11:10 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Rob Herring, Bjorn Helgaas
Cc: Wei Yongjun, linux-omap, linux-pci, kernel-janitors
platform_get_resource() may fail and return NULL, so we should
better check it's return value to avoid a NULL pointer dereference
a bit later in the code.
This is detected by Coccinelle semantic patch.
@@
expression pdev, res, n, t, e, e1, e2;
@@
res = \(platform_get_resource\|platform_get_resource_byname\)(pdev, t, n);
+ if (!res)
+ return -EINVAL;
... when != res == NULL
e = devm_ioremap(e1, res->start, e2);
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 3b0e58f2de58..7a3d12f7e7d9 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -878,6 +878,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
}
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
+ if (!res)
+ return -EINVAL;
+
base = devm_ioremap(dev, res->start, resource_size(res));
if (!base)
return -ENOMEM;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe()
2020-04-27 11:10 [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe() Wei Yongjun
@ 2020-04-27 11:59 ` Vignesh Raghavendra
2020-04-28 13:07 ` Dan Carpenter
2020-04-29 1:50 ` [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname() Wei Yongjun
1 sibling, 1 reply; 7+ messages in thread
From: Vignesh Raghavendra @ 2020-04-27 11:59 UTC (permalink / raw)
To: Wei Yongjun, Kishon Vijay Abraham I, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas
Cc: linux-omap, linux-pci, kernel-janitors
Hi,
On 27/04/20 4:40 pm, Wei Yongjun wrote:
> platform_get_resource() may fail and return NULL, so we should
> better check it's return value to avoid a NULL pointer dereference
> a bit later in the code.
>
> This is detected by Coccinelle semantic patch.
>
> @@
> expression pdev, res, n, t, e, e1, e2;
> @@
>
> res = \(platform_get_resource\|platform_get_resource_byname\)(pdev, t, n);
> + if (!res)
> + return -EINVAL;
> ... when != res == NULL
> e = devm_ioremap(e1, res->start, e2);
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 3b0e58f2de58..7a3d12f7e7d9 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -878,6 +878,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
> }
>
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
> + if (!res)
> + return -EINVAL;
> +
> base = devm_ioremap(dev, res->start, resource_size(res));
I don't see why this should be devm_ioremap(). It should also have been
devm_ioremap_resource() which does the NULL check.
Alternately, how about using devm_platform_ioremap_resource_byname()?
> if (!base)
> return -ENOMEM;
>
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe()
2020-04-27 11:59 ` Vignesh Raghavendra
@ 2020-04-28 13:07 ` Dan Carpenter
2020-04-28 16:15 ` Vignesh Raghavendra
0 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2020-04-28 13:07 UTC (permalink / raw)
To: Vignesh Raghavendra
Cc: Wei Yongjun, Kishon Vijay Abraham I, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, linux-omap, linux-pci,
kernel-janitors
On Mon, Apr 27, 2020 at 05:29:50PM +0530, Vignesh Raghavendra wrote:
> Hi,
>
> On 27/04/20 4:40 pm, Wei Yongjun wrote:
> > platform_get_resource() may fail and return NULL, so we should
> > better check it's return value to avoid a NULL pointer dereference
> > a bit later in the code.
> >
> > This is detected by Coccinelle semantic patch.
> >
> > @@
> > expression pdev, res, n, t, e, e1, e2;
> > @@
> >
> > res = \(platform_get_resource\|platform_get_resource_byname\)(pdev, t, n);
> > + if (!res)
> > + return -EINVAL;
> > ... when != res == NULL
> > e = devm_ioremap(e1, res->start, e2);
> >
> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> > ---
> > drivers/pci/controller/dwc/pci-dra7xx.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> > index 3b0e58f2de58..7a3d12f7e7d9 100644
> > --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> > +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> > @@ -878,6 +878,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
> > }
> >
> > res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
^^^^
> > + if (!res)
> > + return -EINVAL;
> > +
> > base = devm_ioremap(dev, res->start, resource_size(res));
^^^
>
> I don't see why this should be devm_ioremap(). It should also have been
> devm_ioremap_resource() which does the NULL check.
It's different device pointers.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe()
2020-04-28 13:07 ` Dan Carpenter
@ 2020-04-28 16:15 ` Vignesh Raghavendra
2020-04-28 17:23 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Vignesh Raghavendra @ 2020-04-28 16:15 UTC (permalink / raw)
To: Dan Carpenter
Cc: Wei Yongjun, Kishon Vijay Abraham I, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, linux-omap, linux-pci,
kernel-janitors
On 28/04/20 6:37 pm, Dan Carpenter wrote:
> On Mon, Apr 27, 2020 at 05:29:50PM +0530, Vignesh Raghavendra wrote:
>> Hi,
>>
>> On 27/04/20 4:40 pm, Wei Yongjun wrote:
>>> platform_get_resource() may fail and return NULL, so we should
>>> better check it's return value to avoid a NULL pointer dereference
>>> a bit later in the code.
>>>
>>> This is detected by Coccinelle semantic patch.
>>>
>>> @@
>>> expression pdev, res, n, t, e, e1, e2;
>>> @@
>>>
>>> res = \(platform_get_resource\|platform_get_resource_byname\)(pdev, t, n);
>>> + if (!res)
>>> + return -EINVAL;
>>> ... when != res == NULL
>>> e = devm_ioremap(e1, res->start, e2);
>>>
>>> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
>>> ---
>>> drivers/pci/controller/dwc/pci-dra7xx.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
>>> index 3b0e58f2de58..7a3d12f7e7d9 100644
>>> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
>>> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
>>> @@ -878,6 +878,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
>>> }
>>>
>>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
> ^^^^
>
>>> + if (!res)
>>> + return -EINVAL;
>>> +
>>> base = devm_ioremap(dev, res->start, resource_size(res));
> ^^^
>>
>> I don't see why this should be devm_ioremap(). It should also have been
>> devm_ioremap_resource() which does the NULL check.
>
> It's different device pointers.
>
Sorry, I don't understand this comment... Currently we have:
static int __init dra7xx_pcie_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
...
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
base = devm_ioremap(dev, res->start, resource_size(res));
if (!base)
return -ENOMEM;
Instead of the proposed patch, what I am asking is:
base = devm_platform_ioremap_resource_byname(pdev, "ti_conf");
if (IS_ERR(base))
return PTR_ERR(base);
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe()
2020-04-28 16:15 ` Vignesh Raghavendra
@ 2020-04-28 17:23 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2020-04-28 17:23 UTC (permalink / raw)
To: Vignesh Raghavendra
Cc: Wei Yongjun, Kishon Vijay Abraham I, Lorenzo Pieralisi,
Rob Herring, Bjorn Helgaas, linux-omap, linux-pci,
kernel-janitors
Oh. Crap. I'm sorry. I misread the code.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname()
2020-04-27 11:10 [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe() Wei Yongjun
2020-04-27 11:59 ` Vignesh Raghavendra
@ 2020-04-29 1:50 ` Wei Yongjun
2020-05-05 10:54 ` Lorenzo Pieralisi
1 sibling, 1 reply; 7+ messages in thread
From: Wei Yongjun @ 2020-04-29 1:50 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Lorenzo Pieralisi, Rob Herring,
Bjorn Helgaas, Vignesh Raghavendra, Dan Carpenter
Cc: Wei Yongjun, linux-omap, linux-pci, kernel-janitors
platform_get_resource() may fail and return NULL, so we should better
check it's return value to avoid a NULL pointer dereference a bit later
in the code. Fix it to use devm_platform_ioremap_resource_byname()
instead of calling platform_get_resource_byname() and devm_ioremap().
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
v1 -> v2: use devm_platform_ioremap_resource_byname, suggest by Vignesh
---
drivers/pci/controller/dwc/pci-dra7xx.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 3b0e58f2de58..6184ebc9392d 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -840,7 +840,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
struct phy **phy;
struct device_link **link;
void __iomem *base;
- struct resource *res;
struct dw_pcie *pci;
struct dra7xx_pcie *dra7xx;
struct device *dev = &pdev->dev;
@@ -877,10 +876,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
return irq;
}
- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
- base = devm_ioremap(dev, res->start, resource_size(res));
- if (!base)
- return -ENOMEM;
+ base = devm_platform_ioremap_resource_byname(pdev, "ti_conf");
+ if (IS_ERR(base))
+ return PTR_ERR(base);
phy_count = of_property_count_strings(np, "phy-names");
if (phy_count < 0) {
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname()
2020-04-29 1:50 ` [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname() Wei Yongjun
@ 2020-05-05 10:54 ` Lorenzo Pieralisi
0 siblings, 0 replies; 7+ messages in thread
From: Lorenzo Pieralisi @ 2020-05-05 10:54 UTC (permalink / raw)
To: Wei Yongjun
Cc: Kishon Vijay Abraham I, Rob Herring, Bjorn Helgaas,
Vignesh Raghavendra, Dan Carpenter, linux-omap, linux-pci,
kernel-janitors
On Wed, Apr 29, 2020 at 01:50:27AM +0000, Wei Yongjun wrote:
> platform_get_resource() may fail and return NULL, so we should better
> check it's return value to avoid a NULL pointer dereference a bit later
> in the code. Fix it to use devm_platform_ioremap_resource_byname()
> instead of calling platform_get_resource_byname() and devm_ioremap().
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> v1 -> v2: use devm_platform_ioremap_resource_byname, suggest by Vignesh
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
Applied to pci/dwc, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 3b0e58f2de58..6184ebc9392d 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -840,7 +840,6 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
> struct phy **phy;
> struct device_link **link;
> void __iomem *base;
> - struct resource *res;
> struct dw_pcie *pci;
> struct dra7xx_pcie *dra7xx;
> struct device *dev = &pdev->dev;
> @@ -877,10 +876,9 @@ static int __init dra7xx_pcie_probe(struct platform_device *pdev)
> return irq;
> }
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ti_conf");
> - base = devm_ioremap(dev, res->start, resource_size(res));
> - if (!base)
> - return -ENOMEM;
> + base = devm_platform_ioremap_resource_byname(pdev, "ti_conf");
> + if (IS_ERR(base))
> + return PTR_ERR(base);
>
> phy_count = of_property_count_strings(np, "phy-names");
> if (phy_count < 0) {
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-05 10:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-27 11:10 [PATCH -next] PCI: dwc: pci-dra7xx: Fix potential NULL dereference in dra7xx_pcie_probe() Wei Yongjun
2020-04-27 11:59 ` Vignesh Raghavendra
2020-04-28 13:07 ` Dan Carpenter
2020-04-28 16:15 ` Vignesh Raghavendra
2020-04-28 17:23 ` Dan Carpenter
2020-04-29 1:50 ` [PATCH -next v2] PCI: dwc: pci-dra7xx: use devm_platform_ioremap_resource_byname() Wei Yongjun
2020-05-05 10:54 ` Lorenzo Pieralisi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).