Re: [Patch next] octeontx2-pf: fix potential double free in rvu_rep_create()

[Abdun Nihaal]

On Sun, Apr 13, 2025 at 02:16:39PM +0800, cxxz16 wrote:
> If either rvu_rep_devlink_port_register() or register_netdev() fails,
> the function frees ndev using free_netdev(ndev) before jumping to 
> the 'exit:' label. However, in the 'exit:' section, the function 
> iterates over priv->reps[] and again frees rep->netdev, which points 
> to the same ndev.
> 
> This results in a potential double free of the same netdev pointer,
> which can cause memory corruption or crashes.
> 
> To fix this, avoid calling free_netdev(ndev) before jumping to 'exit:'.
> The cleanup logic at 'exit:' should handle the freeing safely.
> 
> Signed-off-by: cxxz16 <990492108@xxxxxx>
> ---
>  drivers/net/ethernet/marvell/octeontx2/nic/rep.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> index 04e08e06f30f..de9a50f2fc39 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> @@ -681,7 +681,6 @@ int rvu_rep_create(struct otx2_nic *priv, struct netlink_ext_ack *extack)
>  		eth_hw_addr_random(ndev);
>  		err = rvu_rep_devlink_port_register(rep);
>  		if (err) {
> -			free_netdev(ndev);
>  			goto exit;
>  		}
>  
> @@ -691,7 +690,6 @@ int rvu_rep_create(struct otx2_nic *priv, struct netlink_ext_ack *extack)
>  			NL_SET_ERR_MSG_MOD(extack,
>  					   "PFVF representor registration failed");
>  			rvu_rep_devlink_port_unregister(rep);
> -			free_netdev(ndev);
>  			goto exit;
>  		}

There is no potential double free here. If you notice the loop at the
exit label has a predecrement (--rep_id) so if rep_id is say 7 when the
rvu_rep_devlink_port_unregister or register_netdev fails, then the loop
at the exit label would free from rep_id = 6 to 0. And so the
free_netdev calls on those two lines are required.

exit:
	while (--rep_id >= 0) {
		rep = priv->reps[rep_id];
		unregister_netdev(rep->netdev);
		rvu_rep_devlink_port_unregister(rep);
		free_netdev(rep->netdev);
	}

(De)allocations in loops are quite tricky.

Nacked-by: Abdun Nihaal <abdun.nihaal@xxxxxxxxx>