Thread (6 messages) 6 messages, 6 authors, 2021-05-03

Re: [PATCH][next] PCI: mediatek-gen3: Add missing null pointer check

From: Dan Carpenter <hidden>
Date: 2021-05-03 07:04:38
Also in: kernel-janitors, linux-mediatek, linux-pci, lkml

On Fri, Apr 30, 2021 at 11:34:50AM -0500, Bjorn Helgaas wrote:
On Fri, Apr 30, 2021 at 09:47:06AM +0200, Christophe JAILLET wrote:
quoted
Le 29/04/2021 à 13:00, Colin King a écrit :
quoted
From: Colin Ian King <redacted>

The call to platform_get_resource_byname can potentially return null, so
add a null pointer check to avoid a null pointer dereference issue.

Addresses-Coverity: ("Dereference null return")
Fixes: 441903d9e8f0 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192")
Signed-off-by: Colin Ian King <redacted>
---
  drivers/pci/controller/pcie-mediatek-gen3.c | 2 ++
  1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 20165e4a75b2..3c5b97716d40 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -721,6 +721,8 @@ static int mtk_pcie_parse_port(struct mtk_pcie_port *port)
  	int ret;
  	regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pcie-mac");
+	if (!regs)
+		return -EINVAL;
  	port->base = devm_ioremap_resource(dev, regs);
  	if (IS_ERR(port->base)) {
  		dev_err(dev, "failed to map register base\n");
Nitpick:
   Using 'devm_platform_ioremap_resource_byname' is slightly less verbose
and should please Coverity.
Not a nitpick at all.  Jianjun is correct that devm_ioremap_resource()
does check "regs" for NULL and it fails gracefully before trying to
dereference it, so the extra check shouldn't be needed.  And most
cases in drivers/pci/ look like this, without the extra check:

  res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
  base = devm_ioremap_resource(dev, res);
  if (IS_ERR(base))
    ...

If devm_platform_ioremap_resource_byname() keeps Coverity happy, I
think that's what we should be doing across drivers/pci/.  Coverity
false positives are a hassle.
Smatch knows that devm_ioremap_resource() will return ERR_PTR(-EINVAL)
when we pass it a NULL.  ;)

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help