linux-kernel - Re: [PATCH net-next v7 1/3] net: ethtool: Introduce per-PHY DUMP operations

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: <20250430083220.5fba6c32@device-130.home>
Date: Wed, 30 Apr 2025 08:32:20 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, Andrew Lunn <andrew@...n.ch>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Heiner Kallweit
 <hkallweit1@...il.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, thomas.petazzoni@...tlin.com,
 linux-arm-kernel@...ts.infradead.org, Christophe Leroy
 <christophe.leroy@...roup.eu>, Herve Codina <herve.codina@...tlin.com>,
 Florian Fainelli <f.fainelli@...il.com>, Russell King
 <linux@...linux.org.uk>, Vladimir Oltean <vladimir.oltean@....com>,
 Köry Maincent <kory.maincent@...tlin.com>, Oleksij Rempel
 <o.rempel@...gutronix.de>, Simon Horman <horms@...nel.org>, Romain Gantois
 <romain.gantois@...tlin.com>, Piergiorgio Beruto
 <piergiorgio.beruto@...il.com>
Subject: Re: [PATCH net-next v7 1/3] net: ethtool: Introduce per-PHY DUMP
 operations

Hi Jakub,

On Thu, 24 Apr 2025 18:03:33 -0700
Jakub Kicinski <kuba@...nel.org> wrote:


> > +
> > +/* perphy ->dumpit() handler for GET requests. */
> > +static int ethnl_perphy_dumpit(struct sk_buff *skb,
> > +			       struct netlink_callback *cb)
> > +{
> > +	struct ethnl_perphy_dump_ctx *ctx = ethnl_perphy_dump_context(cb);
> > +	struct ethnl_dump_ctx *ethnl_ctx = &ctx->ethnl_ctx;
> > +	int ret = 0;
> > +
> > +	if (ethnl_ctx->req_info->dev) {
> > +		ret = ethnl_perphy_dump_one_dev(skb, ethnl_ctx->req_info->dev,
> > +						ctx, genl_info_dump(cb));
> > +		if (ret < 0 && ret != -EOPNOTSUPP && likely(skb->len))
> > +			ret = skb->len;
> > +
> > +		netdev_put(ethnl_ctx->req_info->dev,
> > +			   &ethnl_ctx->req_info->dev_tracker);  
> 
> You have to release this in .done
> dumpit gets called multiple times until we run out of objects to dump.
> OTOH user may close the socket without finishing the dump operation.
> So all .dumpit implementations must be "balanced". The only state we
> should touch in them is the dump context to know where to pick up from
> next time.

Sorry for the delayed answer, I'm still wrapping my head around all
this. calling netdev_put in dumpit() is not correct, but won't moving
this into .done() make us subject to the scenario you described to me
in another mail where userspace would stall the dump operation by not
calling recv() ?

Maybe the safest path, as you mention in the other thread, would be to
store the requesting ifindex in .start(), call netdev_put() in start()
as well, and hold/dump/put in each .dumpit() then.

Maxime