linux-kernel - Re: [PATCH net] net: ethtool: Don't call .cleanup_data when prepare

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: <jbrczz3ylkccyw332pkrzpphe3nrhkcfx3pup6adiufrrwe5s7@zfpk343qajtc>
Date: Fri, 4 Apr 2025 23:45:37 +0200
From: Michal Kubecek <mkubecek@...e.cz>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: davem@...emloft.net, Andrew Lunn <andrew@...n.ch>, 
	Jakub Kicinski <kuba@...nel.org>, Eric Dumazet <edumazet@...gle.com>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	thomas.petazzoni@...tlin.com, Simon Horman <horms@...nel.org>, 
	Florian Fainelli <f.fainelli@...il.com>, Kory Maincent <kory.maincent@...tlin.com>
Subject: Re: [PATCH net] net: ethtool: Don't call .cleanup_data when
 prepare_data fails

On Thu, Apr 03, 2025 at 03:24:46PM +0200, Maxime Chevallier wrote:
> There's a consistent pattern where the .cleanup_data() callback is
> called when .prepare_data() fails, when it should really be called to
> clean after a successfull .prepare_data() as per the documentation.

Agreed. The rationale is that only ->prepare_data() callback knows
what exactly failed and what needs to be reverted. And it makes much
more sense for it to do the necessary cleanup than to provide enough
information for it to be done elsewhere. Except, of course, the simple
cases where ->cleanup() is just a bunch of kfree() calls with zeroing
those pointers so that it can be called repeatedly.

> 
> Rewrite the error-handling paths to make sure we don't cleanup
> un-prepared data.
> 
> Fixes: 728480f12442 ("ethtool: default handlers for GET requests")
> Reviewed-by: Kory Maincent <kory.maincent@...tlin.com>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>

Reviewed-by: Michal Kubecek <mkubecek@...e.cz>

Michal

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)