| Message ID | 20240911134623.1739633-1-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: ethtool: phy: Clear the netdev context pointer for DUMP requests | expand |
On Wed, 11 Sep 2024 15:46:21 +0200 Maxime Chevallier wrote: > + /* Clear the context netdev pointer so avoid a netdev_put from > + * the .done() callback > + */ > + ctx->phy_req_info->base.dev = NULL; Why do we assign to req_base.dev in the first place? req is for the parsed request in my mind, and I don't see anything in the PHY dump handlers actually using dev?
Hello Jakub, On Thu, 12 Sep 2024 20:44:38 -0700 Jakub Kicinski <kuba@kernel.org> wrote: > On Wed, 11 Sep 2024 15:46:21 +0200 Maxime Chevallier wrote: > > + /* Clear the context netdev pointer so avoid a netdev_put from > > + * the .done() callback > > + */ > > + ctx->phy_req_info->base.dev = NULL; > > Why do we assign to req_base.dev in the first place? > req is for the parsed request in my mind, and I don't > see anything in the PHY dump handlers actually using dev? After reading the code back, that's true. It's just a leftover from when I hadn't considered that dumps could be interrupted/resumed. Let me clean that up then. Thanks for the review Jakub, Maxime
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c index 560dd039c662..99d2a8b6144c 100644 --- a/net/ethtool/phy.c +++ b/net/ethtool/phy.c @@ -301,6 +301,11 @@ int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb) ctx->phy_index = 0; } + + /* Clear the context netdev pointer so avoid a netdev_put from + * the .done() callback + */ + ctx->phy_req_info->base.dev = NULL; } rtnl_unlock();
The context info allows continuing DUMP requests, shall they fill the netlink buffer. When performing unfiltered dump request, clear the dev pointer in the context at the end of the dump, avoiding the release of the netdevice. In the case of a filtered DUMP, the refcount for the netdev was held when parsing the header, and is released in the .done() callback. Reported-by: syzbot+e9ed4e4368d450c8f9db@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/000000000000d3bf150621d361a7@google.com/ Fixes: 17194be4c8e1 ("net: ethtool: Introduce a command to list PHYs on an interface") Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com> --- net/ethtool/phy.c | 5 +++++ 1 file changed, 5 insertions(+)