linux-kernel - Re: [PATCH net v2 1/2] octeontx2-pf: fix netdev memory leak 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: <116fc5cb-cc46-4e0f-9990-499ae7ef90ee@stanley.mountain>
Date: Wed, 18 Dec 2024 20:51:53 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Markus Elfring <Markus.Elfring@....de>
Cc: Harshit Mogalapalli <harshit.m.mogalapalli@...cle.com>,
	netdev@...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>,
	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>, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org, Dan Carpenter <error27@...il.com>,
	Przemek Kitszel <przemyslaw.kitszel@...el.com>
Subject: Re: [PATCH net v2 1/2] octeontx2-pf: fix netdev memory leak in
 rvu_rep_create()

On Wed, Dec 18, 2024 at 06:38:25PM +0100, Markus Elfring wrote:
> > When rvu_rep_devlink_port_register() fails, free_netdev(ndev) for this
> > incomplete iteration before going to "exit:" label.
> 
> 
> …
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
> > @@ -680,8 +680,10 @@ int rvu_rep_create(struct otx2_nic *priv, struct netlink_ext_ack *extack)
> >  		ndev->features |= ndev->hw_features;
> >  		eth_hw_addr_random(ndev);
> >  		err = rvu_rep_devlink_port_register(rep);
> > -		if (err)
> > +		if (err) {
> > +			free_netdev(ndev);
> >  			goto exit;
> > +		}
> >
> >  		SET_NETDEV_DEVLINK_PORT(ndev, &rep->dl_port);
> …
> 
> I suggest to add another jump target instead so that a bit of exception handling
> can be better reused at the end of this function implementation.
> 

When you're cleaning up from inside a loop, then the best practices is
to clean up partial iterations before the goto and then clean up whole
iterations in the unwind ladder.  So this patch is better the way that
Harshit his written it.

regards,
dan carpenter