2021-03-14 18:12:46

by Tong Zhang

Subject: [PATCH] net: arcnet: com20020 fix error handling

There are two issues when handling error case in com20020pci_probe()

1. priv might be not initialized yet when calling com20020pci_remove()
from com20020pci_probe(), since the priv is set at the very last but it
can jump to error handling in the middle and priv remains NULL.
2. memory leak - the net device is allocated in alloc_arcdev but not
properly released if error happens in the middle of the big for loop

[ 1.529110] BUG: kernel NULL pointer dereference, address: 0000000000000008
[ 1.531447] RIP: 0010:com20020pci_remove+0x15/0x60 [com20020_pci]
[ 1.536805] Call Trace:
[ 1.536939] com20020pci_probe+0x3f2/0x48c [com20020_pci]
[ 1.537226] local_pci_probe+0x48/0x80
[ 1.539918] com20020pci_init+0x3f/0x1000 [com20020_pci]

Signed-off-by: Tong Zhang <[email protected]>
---
drivers/net/arcnet/com20020-pci.c | 34 +++++++++++++++++--------------
1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index 8bdc44b7e09a..3c8f665c1558 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -127,6 +127,8 @@ static int com20020pci_probe(struct pci_dev *pdev,
int i, ioaddr, ret;
struct resource *r;

+ ret = 0;
+
if (pci_enable_device(pdev))
return -EIO;

@@ -139,6 +141,8 @@ static int com20020pci_probe(struct pci_dev *pdev,
priv->ci = ci;
mm = &ci->misc_map;

+ pci_set_drvdata(pdev, priv);
+
INIT_LIST_HEAD(&priv->list_dev);

if (mm->size) {
@@ -161,7 +165,7 @@ static int com20020pci_probe(struct pci_dev *pdev,
dev = alloc_arcdev(device);
if (!dev) {
ret = -ENOMEM;
- goto out_port;
+ break;
}
dev->dev_port = i;

@@ -178,7 +182,7 @@ static int com20020pci_probe(struct pci_dev *pdev,
pr_err("IO region %xh-%xh already allocated\n",
ioaddr, ioaddr + cm->size - 1);
ret = -EBUSY;
- goto out_port;
+ goto err_free_arcdev;
}

/* Dummy access after Reset
@@ -216,18 +220,18 @@ static int com20020pci_probe(struct pci_dev *pdev,
if (arcnet_inb(ioaddr, COM20020_REG_R_STATUS) == 0xFF) {
pr_err("IO address %Xh is empty!\n", ioaddr);
ret = -EIO;
- goto out_port;
+ goto err_free_arcdev;
}
if (com20020_check(dev)) {
ret = -EIO;
- goto out_port;
+ goto err_free_arcdev;
}

card = devm_kzalloc(&pdev->dev, sizeof(struct com20020_dev),
GFP_KERNEL);
if (!card) {
ret = -ENOMEM;
- goto out_port;
+ goto err_free_arcdev;
}

card->index = i;
@@ -253,29 +257,29 @@ static int com20020pci_probe(struct pci_dev *pdev,

ret = devm_led_classdev_register(&pdev->dev, &card->tx_led);
if (ret)
- goto out_port;
+ goto err_free_arcdev;

ret = devm_led_classdev_register(&pdev->dev, &card->recon_led);
if (ret)
- goto out_port;
+ goto err_free_arcdev;

dev_set_drvdata(&dev->dev, card);

ret = com20020_found(dev, IRQF_SHARED);
if (ret)
- goto out_port;
+ goto err_free_arcdev;

devm_arcnet_led_init(dev, dev->dev_id, i);

list_add(&card->list, &priv->list_dev);
- }
+ continue;

- pci_set_drvdata(pdev, priv);
-
- return 0;
-
-out_port:
- com20020pci_remove(pdev);
+err_free_arcdev:
+ free_arcdev(dev);
+ break;
+ }
+ if (ret)
+ com20020pci_remove(pdev);
return ret;
}

--
2.25.1

2021-03-14 21:38:47

by patchwork-bot+netdevbpf

Subject: Re: [PATCH] net: arcnet: com20020 fix error handling

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Sun, 14 Mar 2021 14:08:36 -0400 you wrote:
> There are two issues when handling error case in com20020pci_probe()
>
> 1. priv might be not initialized yet when calling com20020pci_remove()
> from com20020pci_probe(), since the priv is set at the very last but it
> can jump to error handling in the middle and priv remains NULL.
> 2. memory leak - the net device is allocated in alloc_arcdev but not
> properly released if error happens in the middle of the big for loop
>
> [...]

Here is the summary with links:
- net: arcnet: com20020 fix error handling
https://git.kernel.org/netdev/net/c/6577b9a551ae

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html