| Message ID | 1481479310-2698-1-git-send-email-arvind.yadav.cs@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 12/11/2016 07:01 PM, Arvind Yadav wrote: > Here, If devm_ioremap will fail. It will return NULL. > Kernel can run into a NULL-pointer dereference. > > Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> > --- > drivers/mtd/devices/docg3.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c > index b833e6c..013b5b9 100644 > --- a/drivers/mtd/devices/docg3.c > +++ b/drivers/mtd/devices/docg3.c > @@ -2083,9 +2083,12 @@ static int __init docg3_probe(struct platform_device *pdev) > dev_err(dev, "No I/O memory resource defined\n"); > return ret; > } > - base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); > > ret = -ENOMEM; > + base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); > + if (!base) > + return ret; I think return -ENOMEM right away won't hurt here. Also, dev_err() explaining the failure would be nice to add. Thanks! > cascade = devm_kzalloc(dev, sizeof(*cascade) * DOC_MAX_NBFLOORS, > GFP_KERNEL); > if (!cascade) >
Yes, We are returning -ENOMEM, ret is initialized to -ENOMEM. As per your concern, I have added dev_err failure message. Thanks -Arvind On Monday 12 December 2016 12:45 AM, Marek Vasut wrote: > On 12/11/2016 07:01 PM, Arvind Yadav wrote: >> Here, If devm_ioremap will fail. It will return NULL. >> Kernel can run into a NULL-pointer dereference. >> >> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> >> --- >> drivers/mtd/devices/docg3.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c >> index b833e6c..013b5b9 100644 >> --- a/drivers/mtd/devices/docg3.c >> +++ b/drivers/mtd/devices/docg3.c >> @@ -2083,9 +2083,12 @@ static int __init docg3_probe(struct platform_device *pdev) >> dev_err(dev, "No I/O memory resource defined\n"); >> return ret; >> } >> - base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); >> >> ret = -ENOMEM; >> + base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); >> + if (!base) >> + return ret; > I think return -ENOMEM right away won't hurt here. Also, dev_err() > explaining the failure would be nice to add. > > Thanks! > >> cascade = devm_kzalloc(dev, sizeof(*cascade) * DOC_MAX_NBFLOORS, >> GFP_KERNEL); >> if (!cascade) >> >
On Sun, Dec 11, 2016 at 5:15 PM, Marek Vasut <marek.vasut@gmail.com> wrote: >> ret = -ENOMEM; >> + base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); >> + if (!base) >> + return ret; > > I think return -ENOMEM right away won't hurt here. Also, dev_err() > explaining the failure would be nice to add. There is no need to add dev_err() message as the mm core will complain loudly if devm_ioremap() fails.
diff --git a/drivers/mtd/devices/docg3.c b/drivers/mtd/devices/docg3.c index b833e6c..013b5b9 100644 --- a/drivers/mtd/devices/docg3.c +++ b/drivers/mtd/devices/docg3.c @@ -2083,9 +2083,12 @@ static int __init docg3_probe(struct platform_device *pdev) dev_err(dev, "No I/O memory resource defined\n"); return ret; } - base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); ret = -ENOMEM; + base = devm_ioremap(dev, ress->start, DOC_IOSPACE_SIZE); + if (!base) + return ret; + cascade = devm_kzalloc(dev, sizeof(*cascade) * DOC_MAX_NBFLOORS, GFP_KERNEL); if (!cascade)
Here, If devm_ioremap will fail. It will return NULL. Kernel can run into a NULL-pointer dereference. Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> --- drivers/mtd/devices/docg3.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)