[net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[Subbaraya Sundeep]

rvu_mbox_init function makes use of error path for
freeing memory which are local to the function in
both success and failure conditions. This is unusual hence
fix it by returning zero on success. With new cn20k code this
is freeing valid memory in success case also.

Fixes: e53ee4acb220 ("octeontx2-af: CN20k basic mbox operations and structures")
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
index bfee71f4..7e538ee 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c
@@ -2573,7 +2573,11 @@ static int rvu_mbox_init(struct rvu *rvu, struct mbox_wq_info *mw,
 		mwork->rvu = rvu;
 		INIT_WORK(&mwork->work, mbox_up_handler);
 	}
-	goto free_regions;
+
+	kfree(mbox_regions);
+	bitmap_free(pf_bmap);
+
+	return 0;
 
 exit:
 	destroy_workqueue(mw->mbox_wq);
-- 
2.7.4

Re: [net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[Simon Horman]

On Wed, Jun 18, 2025 at 07:27:16PM +0530, Subbaraya Sundeep wrote:
> rvu_mbox_init function makes use of error path for
> freeing memory which are local to the function in
> both success and failure conditions. This is unusual hence
> fix it by returning zero on success. With new cn20k code this
> is freeing valid memory in success case also.
> 
> Fixes: e53ee4acb220 ("octeontx2-af: CN20k basic mbox operations and structures")
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Although I don't think the problem is introduced by this patch
with it applied Smatch notices that the following code, around line 2528,
which jumps to free_regions does so with err uninitialised. This is a
problem because the jump will result in the function returning err.

	switch (type) {
	case TYPE_AFPF
	...
	default:

		goto free_regions;
	}

...

Re: [net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[Subbaraya Sundeep]

On 2025-06-18 at 19:43:01, Simon Horman (horms@kernel.org) wrote:
> On Wed, Jun 18, 2025 at 07:27:16PM +0530, Subbaraya Sundeep wrote:
> > rvu_mbox_init function makes use of error path for
> > freeing memory which are local to the function in
> > both success and failure conditions. This is unusual hence
> > fix it by returning zero on success. With new cn20k code this
> > is freeing valid memory in success case also.
> > 
> > Fixes: e53ee4acb220 ("octeontx2-af: CN20k basic mbox operations and structures")
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> Although I don't think the problem is introduced by this patch
> with it applied Smatch notices that the following code, around line 2528,
> which jumps to free_regions does so with err uninitialised. This is a
> problem because the jump will result in the function returning err.

Thanks Simon for review. I will send fix for Smatch error in
separate patch later.

Sundeep
> 
> 	switch (type) {
> 	case TYPE_AFPF
> 	...
> 	default:
> 
> 		goto free_regions;
> 	}
> 
> ...

Re: [net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[Simon Horman]

On Fri, Jun 20, 2025 at 11:14:43AM +0000, Subbaraya Sundeep wrote:
> On 2025-06-18 at 19:43:01, Simon Horman (horms@kernel.org) wrote:
> > On Wed, Jun 18, 2025 at 07:27:16PM +0530, Subbaraya Sundeep wrote:
> > > rvu_mbox_init function makes use of error path for
> > > freeing memory which are local to the function in
> > > both success and failure conditions. This is unusual hence
> > > fix it by returning zero on success. With new cn20k code this
> > > is freeing valid memory in success case also.
> > > 
> > > Fixes: e53ee4acb220 ("octeontx2-af: CN20k basic mbox operations and structures")
> > > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> > 
> > Reviewed-by: Simon Horman <horms@kernel.org>
> > 
> > Although I don't think the problem is introduced by this patch
> > with it applied Smatch notices that the following code, around line 2528,
> > which jumps to free_regions does so with err uninitialised. This is a
> > problem because the jump will result in the function returning err.
> 
> Thanks Simon for review. I will send fix for Smatch error in
> separate patch later.

Excellent, thanks.

Re: [net-next PATCH] octeontx2-af: Fix rvu_mbox_init return path

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Wed, 18 Jun 2025 19:27:16 +0530 you wrote:
> rvu_mbox_init function makes use of error path for
> freeing memory which are local to the function in
> both success and failure conditions. This is unusual hence
> fix it by returning zero on success. With new cn20k code this
> is freeing valid memory in success case also.
> 
> Fixes: e53ee4acb220 ("octeontx2-af: CN20k basic mbox operations and structures")
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> 
> [...]

Here is the summary with links:
  - [net-next] octeontx2-af: Fix rvu_mbox_init return path
    https://git.kernel.org/netdev/net-next/c/0289c51f889e

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