[PATCH net-next] net: thunderx: fix double free error

From: Aleksey Makarov
Date: Thu Nov 09 2017 - 06:59:29 EST


This patch fixes an error in memory allocation/freeing in
ThunderX PF driver.

I moved the allocation to the probe() function and made it managed.

>From the Colin's email:

While running static analysis on linux-next with CoverityScan I found 3
double free errors in the Cavium thunder driver.

The issue occurs on the err_disable_device: label of function nic_probe
when nic_free_lmacmem(nic) is called and a double free occurs on
nic->duplex, nic->link and nic->speed. This occurs when nic_init_hw()
fails:

/* Initialize hardware */
err = nic_init_hw(nic);
if (err)
goto err_release_regions;

nic_init_hw() calls nic_get_hw_info() and this calls nic_free_lmacmem()
if any of the allocations fail. This free'ing occurs again by the call
to nic_free_lmacmem() on the err_release_regions exit path in nic_probe().

Reported-by: Colin Ian King <colin.king@xxxxxxxxxxxxx>
Signed-off-by: Aleksey Makarov <aleksey.makarov@xxxxxxxxxx>
---
drivers/net/ethernet/cavium/thunder/nic_main.c | 82 ++++++++++----------------
1 file changed, 30 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c b/drivers/net/ethernet/cavium/thunder/nic_main.c
index c12ab7f1adb9..26309e209217 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -361,17 +361,8 @@ static void nic_set_lmac_vf_mapping(struct nicpf *nic)
}
}

-static void nic_free_lmacmem(struct nicpf *nic)
+static void nic_get_hw_info(struct nicpf *nic)
{
- kfree(nic->vf_lmac_map);
- kfree(nic->link);
- kfree(nic->duplex);
- kfree(nic->speed);
-}
-
-static int nic_get_hw_info(struct nicpf *nic)
-{
- u8 max_lmac;
u16 sdevid;
struct hw_info *hw = nic->hw;

@@ -419,41 +410,16 @@ static int nic_get_hw_info(struct nicpf *nic)
break;
}
hw->tl4_cnt = MAX_QUEUES_PER_QSET * pci_sriov_get_totalvfs(nic->pdev);
-
- /* Allocate memory for LMAC tracking elements */
- max_lmac = hw->bgx_cnt * MAX_LMAC_PER_BGX;
- nic->vf_lmac_map = kmalloc_array(max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->vf_lmac_map)
- goto error;
- nic->link = kmalloc_array(max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->link)
- goto error;
- nic->duplex = kmalloc_array(max_lmac, sizeof(u8), GFP_KERNEL);
- if (!nic->duplex)
- goto error;
- nic->speed = kmalloc_array(max_lmac, sizeof(u32), GFP_KERNEL);
- if (!nic->speed)
- goto error;
- return 0;
-
-error:
- nic_free_lmacmem(nic);
- return -ENOMEM;
}

#define BGX0_BLOCK 8
#define BGX1_BLOCK 9

-static int nic_init_hw(struct nicpf *nic)
+static void nic_init_hw(struct nicpf *nic)
{
- int i, err;
+ int i;
u64 cqm_cfg;

- /* Get HW capability info */
- err = nic_get_hw_info(nic);
- if (err)
- return err;
-
/* Enable NIC HW block */
nic_reg_write(nic, NIC_PF_CFG, 0x3);

@@ -507,8 +473,6 @@ static int nic_init_hw(struct nicpf *nic)
cqm_cfg = nic_reg_read(nic, NIC_PF_CQM_CFG);
if (cqm_cfg < NICPF_CQM_MIN_DROP_LEVEL)
nic_reg_write(nic, NIC_PF_CQM_CFG, NICPF_CQM_MIN_DROP_LEVEL);
-
- return 0;
}

/* Channel parse index configuration */
@@ -1325,6 +1289,7 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
{
struct device *dev = &pdev->dev;
struct nicpf *nic;
+ u8 max_lmac;
int err;

BUILD_BUG_ON(sizeof(union nic_mbx) > 16);
@@ -1334,10 +1299,8 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
return -ENOMEM;

nic->hw = devm_kzalloc(dev, sizeof(struct hw_info), GFP_KERNEL);
- if (!nic->hw) {
- devm_kfree(dev, nic);
+ if (!nic->hw)
return -ENOMEM;
- }

pci_set_drvdata(pdev, nic);

@@ -1378,11 +1341,33 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

nic->node = nic_get_node_id(pdev);

- /* Initialize hardware */
- err = nic_init_hw(nic);
- if (err)
+ /* Get HW capability info */
+ nic_get_hw_info(nic);
+
+ /* Allocate memory for LMAC tracking elements */
+ err = -ENOMEM;
+ max_lmac = nic->hw->bgx_cnt * MAX_LMAC_PER_BGX;
+
+ nic->vf_lmac_map = devm_kmalloc_array(dev, max_lmac, sizeof(u8),
+ GFP_KERNEL);
+ if (!nic->vf_lmac_map)
+ goto err_release_regions;
+
+ nic->link = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
+ if (!nic->link)
goto err_release_regions;

+ nic->duplex = devm_kmalloc_array(dev, max_lmac, sizeof(u8), GFP_KERNEL);
+ if (!nic->duplex)
+ goto err_release_regions;
+
+ nic->speed = devm_kmalloc_array(dev, max_lmac, sizeof(u32), GFP_KERNEL);
+ if (!nic->speed)
+ goto err_release_regions;
+
+ /* Initialize hardware */
+ nic_init_hw(nic);
+
nic_set_lmac_vf_mapping(nic);

/* Register interrupts */
@@ -1416,9 +1401,6 @@ static int nic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
err_release_regions:
pci_release_regions(pdev);
err_disable_device:
- nic_free_lmacmem(nic);
- devm_kfree(dev, nic->hw);
- devm_kfree(dev, nic);
pci_disable_device(pdev);
pci_set_drvdata(pdev, NULL);
return err;
@@ -1440,10 +1422,6 @@ static void nic_remove(struct pci_dev *pdev)
nic_unregister_interrupts(nic);
pci_release_regions(pdev);

- nic_free_lmacmem(nic);
- devm_kfree(&pdev->dev, nic->hw);
- devm_kfree(&pdev->dev, nic);
-
pci_disable_device(pdev);
pci_set_drvdata(pdev, NULL);
}
--
2.15.0