Re: [PATCH net] net: ethtool: Don't call .cleanup_data when prepare_data fails

[Michal Kubecek]

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@xxxxxxxxxxx>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@xxxxxxxxxxx>

Reviewed-by: Michal Kubecek <mkubecek@xxxxxxx>

Michal

Attachment: signature.asc
Description: PGP signature