Message ID | 1545115389-173136-1-git-send-email-weiyongjun1@huawei.com (mailing list archive) |
---|---|
State | Mainlined, archived |
Commit | 48504619a65977fb2a4c7182eab637867f1e6548 |
Headers | show |
Series | [-next] clk: imx: fix potential NULL dereference in imx8qxp_lpcg_clk_probe() | expand |
Hi Wei, On Tue, Dec 18, 2018 at 4:36 AM Wei Yongjun <weiyongjun1@huawei.com> wrote: > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > base = devm_ioremap(dev, res->start, resource_size(res)); Wouldn't it be better to use devm_ioremap_resource(), which does the NULL check?
> -----Original Message----- > From: Wei Yongjun [mailto:weiyongjun1@huawei.com] > Sent: Tuesday, December 18, 2018 2:43 PM [...] > 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(e1, res->start, e2); > > Fixes: 1e3121bfe51a ("clk: imx: add imx8qxp lpcg driver") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Thanks for reporting this. Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com> Regards Dong Aisheng > --- > drivers/clk/imx/clk-imx8qxp-lpcg.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c > b/drivers/clk/imx/clk-imx8qxp-lpcg.c > index dcae1dd..8f241a4 100644 > --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c > +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c > @@ -169,6 +169,8 @@ static int imx8qxp_lpcg_clk_probe(struct > platform_device *pdev) > return -ENODEV; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > base = devm_ioremap(dev, res->start, resource_size(res)); > if (!base) > return -ENOMEM; > >
> -----Original Message----- > From: Fabio Estevam [mailto:festevam@gmail.com] > Hi Wei, > > On Tue, Dec 18, 2018 at 4:36 AM Wei Yongjun <weiyongjun1@huawei.com> > wrote: > > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -EINVAL; > > base = devm_ioremap(dev, res->start, resource_size(res)); > > Wouldn't it be better to use devm_ioremap_resource(), which does the NULL > check? No, devm_ioremap_resource will call request_mem_region which we don't want as LPCG memory space may overlap with other modules within the same SS. This is something like the syscon device does: drivers/mfd/syscon.c. Regards Dong Aisheng
Quoting Wei Yongjun (2018-12-17 22:43:09) > 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(e1, res->start, e2); > > Fixes: 1e3121bfe51a ("clk: imx: add imx8qxp lpcg driver") > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> > --- Applied to clk-fixes
diff --git a/drivers/clk/imx/clk-imx8qxp-lpcg.c b/drivers/clk/imx/clk-imx8qxp-lpcg.c index dcae1dd..8f241a4 100644 --- a/drivers/clk/imx/clk-imx8qxp-lpcg.c +++ b/drivers/clk/imx/clk-imx8qxp-lpcg.c @@ -169,6 +169,8 @@ static int imx8qxp_lpcg_clk_probe(struct platform_device *pdev) return -ENODEV; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; base = devm_ioremap(dev, res->start, resource_size(res)); if (!base) return -ENOMEM;
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(e1, res->start, e2); Fixes: 1e3121bfe51a ("clk: imx: add imx8qxp lpcg driver") Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> --- drivers/clk/imx/clk-imx8qxp-lpcg.c | 2 ++ 1 file changed, 2 insertions(+)