| Message ID | 1525657234-88256-1-git-send-email-weiyongjun1@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
Hi Yongjun, On 7 May 2018 at 09:40, Wei Yongjun <weiyongjun1@huawei.com> 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(pdev, t, n); > + if (!res) > + return -EINVAL; > ... when != res == NULL > e = devm_ioremap_nocache(e1, res->start, e2); > > Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Thanks for fixing my mistake. Reviewed-by: Baolin Wang <baolin.wang@linaro.org> > --- > drivers/dma/sprd-dma.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c > index ccdeb8f..dba7a17 100644 > --- a/drivers/dma/sprd-dma.c > +++ b/drivers/dma/sprd-dma.c > @@ -807,6 +807,8 @@ static int sprd_dma_probe(struct platform_device *pdev) > } > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, > resource_size(res)); > if (!sdev->glb_base) >
On Mon, May 07, 2018 at 01:40:34AM +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. > > This is detected by Coccinelle semantic patch. Applied, thanks
On 05/07/2018 03:40 AM, 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(pdev, t, n); > + if (!res) > + return -EINVAL; > ... when != res == NULL > e = devm_ioremap_nocache(e1, res->start, e2); > > Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- > drivers/dma/sprd-dma.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c > index ccdeb8f..dba7a17 100644 > --- a/drivers/dma/sprd-dma.c > +++ b/drivers/dma/sprd-dma.c > @@ -807,6 +807,8 @@ static int sprd_dma_probe(struct platform_device *pdev) > } > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, > resource_size(res)); I think a better improvement would be to replace this with devm_ioremap_resource() which has the NULL pointer check and some other things built-in. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08-05-18, 13:46, Lars-Peter Clausen wrote: > On 05/07/2018 03:40 AM, Wei Yongjun wrote: > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, > > resource_size(res)); > > I think a better improvement would be to replace this with > devm_ioremap_resource() which has the NULL pointer check and some other > things built-in. Sure feel free to send a patch :)
On 05/08/2018 02:05 PM, Vinod Koul wrote: > On 08-05-18, 13:46, Lars-Peter Clausen wrote: >> On 05/07/2018 03:40 AM, Wei Yongjun wrote: >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + if (!res) >>> + return -EINVAL; >>> sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, >>> resource_size(res)); >> >> I think a better improvement would be to replace this with >> devm_ioremap_resource() which has the NULL pointer check and some other >> things built-in. > > Sure feel free to send a patch :) > I prefer to inspire others ;) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 8 May 2018 at 20:10, Lars-Peter Clausen <lars@metafoo.de> wrote: > On 05/08/2018 02:05 PM, Vinod Koul wrote: >> On 08-05-18, 13:46, Lars-Peter Clausen wrote: >>> On 05/07/2018 03:40 AM, Wei Yongjun wrote: >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + if (!res) >>>> + return -EINVAL; >>>> sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, >>>> resource_size(res)); >>> >>> I think a better improvement would be to replace this with >>> devm_ioremap_resource() which has the NULL pointer check and some other >>> things built-in. >> >> Sure feel free to send a patch :) >> > > I prefer to inspire others ;) OK. I will send one incremental patch to fix this issue. Thanks.
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c index ccdeb8f..dba7a17 100644 --- a/drivers/dma/sprd-dma.c +++ b/drivers/dma/sprd-dma.c @@ -807,6 +807,8 @@ static int sprd_dma_probe(struct platform_device *pdev) } res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; sdev->glb_base = devm_ioremap_nocache(&pdev->dev, res->start, resource_size(res)); if (!sdev->glb_base)
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(pdev, t, n); + if (!res) + return -EINVAL; ... when != res == NULL e = devm_ioremap_nocache(e1, res->start, e2); Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver") Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- drivers/dma/sprd-dma.c | 2 ++ 1 file changed, 2 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html