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] [day] [month] [year] [list]

Message-ID: <457o27qvjkgxlread7gvqsf6sz3g2tponkxtmehva6f2msi6xb@giaxsk5o57oe>
Date: Tue, 15 Apr 2025 19:14:11 +0530
From: Abdun Nihaal <abdun.nihaal@...il.com>
To: Markus Elfring <Markus.Elfring@....de>
Cc: 990492108@...com, netdev@...r.kernel.org, 
	LKML <linux-kernel@...r.kernel.org>, Andrew Lunn <andrew+netdev@...n.ch>, 
	Bharat Bhushan <bbhushan2@...vell.com>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Geethasowjanya Akula <gakula@...vell.com>, 
	Hariprasad Kelam <hkelam@...vell.com>, Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Subbaraya Sundeep Bhatta <sbhatta@...vell.com>, Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [Patch next] octeontx2-pf: fix potential double free in
 rvu_rep_create()

On Tue, Apr 15, 2025 at 09:32:07AM +0200, Markus Elfring wrote:
> Would you ever become interested to avoid a duplicate free_netdev(ndev) call
> by using an additional label instead?
> 
> See also:
> [PATCH net v2 1/2] octeontx2-pf: fix netdev memory leak in rvu_rep_create()
> https://lore.kernel.org/netdev/8d54b21b-7ca9-4126-ba13-bbd333d6ba0c@web.de/

As Dan also pointed out (https://lore.kernel.org/netdev/116fc5cb-cc46-4e0f-9990-499ae7ef90ee@stanley.mountain/),
the best practice is to undo the current iteration inside the loop
and then cleanup the previous iterations after the goto label.

I think the idea is that it makes it easier to review. We look at the
loop and can tell that when it jumps to the error label, the current
iteration is cleaned up. And when we look at the error label, we can see
that it cleans up all previous iterations.

Whereas, if we had additional goto labels, we would have to go back and
forth between the loop and the goto labels, to tell if cleanup is
performed correctly.

Regards,
Nihaal