[PATCH net 0/4] octeon_ep: fix several bugs in exception paths

[PATCH net 0/4] octeon_ep: fix several bugs in exception paths

[Ziyang Xuan]

Find several obvious bugs during code review in exception paths. Provide
this patchset to fix them. Not tested, just compiled.

Ziyang Xuan (4):
  octeon_ep: delete unnecessary napi rollback under set_queues_err in
    octep_open()
  octeon_ep: ensure octep_get_link_status() successfully before
    octep_link_up()
  octeon_ep: fix potential memory leak in octep_device_setup()
  octeon_ep: ensure get mac address successfully before
    eth_hw_addr_set()

 .../net/ethernet/marvell/octeon_ep/octep_main.c  | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH net 1/4] octeon_ep: delete unnecessary napi rollback under set_queues_err in octep_open()

[Ziyang Xuan]

octep_napi_add() and octep_napi_enable() are all after
netif_set_real_num_{tx,rx}_queues() in octep_open(), so it is unnecessary
napi rollback under set_queues_err. Delete them to fix it.

Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 9089adcb75f9..7985a748fafe 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -527,8 +527,6 @@ static int octep_open(struct net_device *netdev)
 	return 0;
 
 set_queues_err:
-	octep_napi_disable(oct);
-	octep_napi_delete(oct);
 	octep_clean_irqs(oct);
 setup_irq_err:
 	octep_free_oqs(oct);
-- 
2.25.1

[PATCH net 2/4] octeon_ep: ensure octep_get_link_status() successfully before octep_link_up()

[Ziyang Xuan]

octep_get_link_status() can fail because send mbox message failed, then
octep_get_link_status() will return ret less than 0. Excute octep_link_up()
as long as ret is not equal to 0 in octep_open() now. That is not correct.

The value type of link.state is enum octep_ctrl_net_state. Positive value
represents up. Excute octep_link_up() when ret is bigger than 0.

Fixes: 862cd659a6fb ("octeon_ep: Add driver framework and device initialization")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 7985a748fafe..546bcebf4462 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -521,7 +521,7 @@ static int octep_open(struct net_device *netdev)
 	octep_oq_dbell_init(oct);
 
 	ret = octep_get_link_status(oct);
-	if (ret)
+	if (ret > 0)
 		octep_link_up(netdev);
 
 	return 0;
-- 
2.25.1

[PATCH net 3/4] octeon_ep: fix potential memory leak in octep_device_setup()

[Ziyang Xuan]

When occur unsupported_dev and mbox init errors, it did not free oct->conf
and iounmap() oct->mmio[i].hw_addr. That would trigger memory leak problem.
Add kfree() for oct->conf and iounmap() for oct->mmio[i].hw_addr under
unsupported_dev and mbox init errors to fix the problem.

Fixes: 862cd659a6fb ("octeon_ep: Add driver framework and device initialization")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 546bcebf4462..53f288c32238 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -956,7 +956,7 @@ int octep_device_setup(struct octep_device *oct)
 	ret = octep_ctrl_mbox_init(ctrl_mbox);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to initialize control mbox\n");
-		return -1;
+		goto unsupported_dev;
 	}
 	oct->ctrl_mbox_ifstats_offset = OCTEP_CTRL_MBOX_SZ(ctrl_mbox->h2fq.elem_sz,
 							   ctrl_mbox->h2fq.elem_cnt,
@@ -966,6 +966,10 @@ int octep_device_setup(struct octep_device *oct)
 	return 0;
 
 unsupported_dev:
+	for (i = 0; i < OCTEP_MMIO_REGIONS; i++)
+		iounmap(oct->mmio[i].hw_addr);
+
+	kfree(oct->conf);
 	return -1;
 }
 
-- 
2.25.1

[PATCH net 4/4] octeon_ep: ensure get mac address successfully before eth_hw_addr_set()

[Ziyang Xuan]

octep_get_mac_addr() can fail because send mbox message failed. If this
happens, octep_dev->mac_addr will be zero. It should not continue to
initialize. Add exception handling for octep_get_mac_addr() to fix it.

Fixes: 862cd659a6fb ("octeon_ep: Add driver framework and device initialization")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
---
 drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 53f288c32238..b45dd7f04e21 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -1072,7 +1072,11 @@ static int octep_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netdev->max_mtu = OCTEP_MAX_MTU;
 	netdev->mtu = OCTEP_DEFAULT_MTU;
 
-	octep_get_mac_addr(octep_dev, octep_dev->mac_addr);
+	err = octep_get_mac_addr(octep_dev, octep_dev->mac_addr);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to get mac address\n");
+		goto register_dev_err;
+	}
 	eth_hw_addr_set(netdev, octep_dev->mac_addr);
 
 	err = register_netdev(netdev);
-- 
2.25.1

Re: [PATCH net 0/4] octeon_ep: fix several bugs in exception paths

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 11 Nov 2022 15:08:01 +0800 you wrote:
> Find several obvious bugs during code review in exception paths. Provide
> this patchset to fix them. Not tested, just compiled.
> 
> Ziyang Xuan (4):
>   octeon_ep: delete unnecessary napi rollback under set_queues_err in
>     octep_open()
>   octeon_ep: ensure octep_get_link_status() successfully before
>     octep_link_up()
>   octeon_ep: fix potential memory leak in octep_device_setup()
>   octeon_ep: ensure get mac address successfully before
>     eth_hw_addr_set()
> 
> [...]

Here is the summary with links:
  - [net,1/4] octeon_ep: delete unnecessary napi rollback under set_queues_err in octep_open()
    https://git.kernel.org/netdev/net/c/298b83e180d5
  - [net,2/4] octeon_ep: ensure octep_get_link_status() successfully before octep_link_up()
    https://git.kernel.org/netdev/net/c/9d3ff7131877
  - [net,3/4] octeon_ep: fix potential memory leak in octep_device_setup()
    https://git.kernel.org/netdev/net/c/e4041be97b15
  - [net,4/4] octeon_ep: ensure get mac address successfully before eth_hw_addr_set()
    https://git.kernel.org/netdev/net/c/848ffce2f0c9

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