[PATCH v2 0/2] octeon_ep: Fix the error handling path of octep_request_irqs()

[PATCH v2 0/2] octeon_ep: Fix the error handling path of octep_request_irqs()

[Christophe JAILLET]

I send a small serie to ease review and because I'm sighly less confident with
the 2nd patch.

They are related to the same Fixes: tag, so they obviously could be merged if
it is preferred.

Details on modification of this v2 are given in each patch.

Christophe JAILLET (2):
  octeon_ep: Fix a memory leak in the error handling path of
    octep_request_irqs()
  octeon_ep: Fix irq releasing in the error handling path of
    octep_request_irqs()

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

-- 
2.34.1

[PATCH v2 1/2] octeon_ep: Fix a memory leak in the error handling path of octep_request_irqs()

[Christophe JAILLET]

'oct->non_ioq_irq_names' is not freed in the error handling path of
octep_request_irqs().

Add the missing kfree().

Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Acked-by: Veerasenareddy Burru <vburru@marvell.com>
---
v2: Add Acked-by tag

v1:
    https://lore.kernel.org/all/78dcfbb5d22328bc83edbfc74af10c3625c54087.1652629833.git.christophe.jaillet@wanadoo.fr/
---
 drivers/net/ethernet/marvell/octeon_ep/octep_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index e020c81f3455..6b60a03574a0 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -267,6 +267,8 @@ static int octep_request_irqs(struct octep_device *oct)
 		--i;
 		free_irq(oct->msix_entries[i].vector, oct);
 	}
+	kfree(oct->non_ioq_irq_names);
+	oct->non_ioq_irq_names = NULL;
 alloc_err:
 	return -1;
 }
-- 
2.34.1

[PATCH v2 2/2] octeon_ep: Fix irq releasing in the error handling path of octep_request_irqs()

[Christophe JAILLET]

When taken, the error handling path does not undo correctly what has
already been allocated.

Introduce a new loop index, 'j', in order to simplify the error handling
path and rewrite part of it.
It is now written with the same logic and intermediate variables used
when resources are allocated. This is much more straightforward.

Fixes: 37d79d059606 ("octeon_ep: add Tx/Rx processing and interrupt support")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
v2: Introduce 'j' and use it in the 2nd for loop
    Rewrite the error handling path (Dan Carpenter)

v1:
    https://lore.kernel.org/all/a1b6f082fff4e68007914577961113bc452c8030.1652629833.git.christophe.jaillet@wanadoo.fr/
---
 .../ethernet/marvell/octeon_ep/octep_main.c   | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 6b60a03574a0..a9b82d221780 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -202,7 +202,7 @@ static int octep_request_irqs(struct octep_device *oct)
 	struct msix_entry *msix_entry;
 	char **non_ioq_msix_names;
 	int num_non_ioq_msix;
-	int ret, i;
+	int ret, i, j;
 
 	num_non_ioq_msix = CFG_GET_NON_IOQ_MSIX(oct->conf);
 	non_ioq_msix_names = CFG_GET_NON_IOQ_MSIX_NAMES(oct->conf);
@@ -233,23 +233,23 @@ static int octep_request_irqs(struct octep_device *oct)
 	}
 
 	/* Request IRQs for Tx/Rx queues */
-	for (i = 0; i < oct->num_oqs; i++) {
-		ioq_vector = oct->ioq_vector[i];
-		msix_entry = &oct->msix_entries[i + num_non_ioq_msix];
+	for (j = 0; j < oct->num_oqs; j++) {
+		ioq_vector = oct->ioq_vector[j];
+		msix_entry = &oct->msix_entries[j + num_non_ioq_msix];
 
 		snprintf(ioq_vector->name, sizeof(ioq_vector->name),
-			 "%s-q%d", netdev->name, i);
+			 "%s-q%d", netdev->name, j);
 		ret = request_irq(msix_entry->vector,
 				  octep_ioq_intr_handler, 0,
 				  ioq_vector->name, ioq_vector);
 		if (ret) {
 			netdev_err(netdev,
 				   "request_irq failed for Q-%d; err=%d",
-				   i, ret);
+				   j, ret);
 			goto ioq_irq_err;
 		}
 
-		cpumask_set_cpu(i % num_online_cpus(),
+		cpumask_set_cpu(j % num_online_cpus(),
 				&ioq_vector->affinity_mask);
 		irq_set_affinity_hint(msix_entry->vector,
 				      &ioq_vector->affinity_mask);
@@ -257,10 +257,13 @@ static int octep_request_irqs(struct octep_device *oct)
 
 	return 0;
 ioq_irq_err:
-	while (i > num_non_ioq_msix) {
-		--i;
-		irq_set_affinity_hint(oct->msix_entries[i].vector, NULL);
-		free_irq(oct->msix_entries[i].vector, oct->ioq_vector[i]);
+	while (j) {
+		--j;
+		ioq_vector = oct->ioq_vector[j];
+		msix_entry = &oct->msix_entries[j + num_non_ioq_msix];
+
+		irq_set_affinity_hint(msix_entry->vector, NULL);
+		free_irq(msix_entry->vector, ioq_vector);
 	}
 non_ioq_irq_err:
 	while (i) {
-- 
2.34.1

Re: [PATCH v2 0/2] octeon_ep: Fix the error handling path of octep_request_irqs()

[Dan Carpenter]

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v2 0/2] octeon_ep: Fix the error handling path of octep_request_irqs()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 17 May 2022 22:59:43 +0200 you wrote:
> I send a small serie to ease review and because I'm sighly less confident with
> the 2nd patch.
> 
> They are related to the same Fixes: tag, so they obviously could be merged if
> it is preferred.
> 
> Details on modification of this v2 are given in each patch.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] octeon_ep: Fix a memory leak in the error handling path of octep_request_irqs()
    https://git.kernel.org/netdev/net-next/c/4d3bf6fb5334
  - [v2,2/2] octeon_ep: Fix irq releasing in the error handling path of octep_request_irqs()
    https://git.kernel.org/netdev/net-next/c/3588c189e45a

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