linux-kernel - Re: [Patch next] octeontx2-pf: fix potential double free in rvu_rep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tzi64aergg2ibm622mk54mavjs6vbpdpfeazdbqoyuufa5ispj@wbygyurrsto5>
Date: Tue, 15 Apr 2025 11:33:06 +0530
From: Abdun Nihaal <abdun.nihaal@...il.com>
To: cxxz16 <990492108@...com>
Cc: sgoutham@...vell.com, gakula@...vell.com, sbhatta@...vell.com, 
	hkelam@...vell.com, bbhushan2@...vell.com, netdev@...r.kernel.org, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	andrew+netdev@...n.ch, linux-kernel@...r.kernel.org
Subject: Re: [Patch next] octeontx2-pf: fix potential double free in
 rvu_rep_create()

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@...com>
> ---
>  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@...il.com>