[PATCH net-next] net: fix reference tracker mismanagement in netdev_put_lock()

[PATCH net-next] net: fix reference tracker mismanagement in netdev_put_lock()

[Jakub Kicinski]

dev_put() releases a reference which didn't have a tracker.
References without a tracker are accounted in the tracking
code as "no_tracker". We can't free the tracker and then
call dev_put(). The references themselves will be fine
but the tracking code will think it's a double-release:

  refcount_t: decrement hit 0; leaking memory.

IOW commit under fixes confused dev_put() (release never tracked
reference) with __dev_put() (just release the reference, skipping
the reference tracking infra).

Since __netdev_put_lock() uses dev_put() we can't feed a previously
tracked netdev ref into it. Let's flip things around.
netdev_put(dev, NULL) is the same as dev_put(dev) so make
netdev_put_lock() the real function and have __netdev_put_lock()
feed it a NULL tracker for all the cases that were untracked.

Fixes: d04686d9bc86 ("net: Implement netdev_nl_queue_create_doit")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: dw@davidwei.uk
CC: skhawaja@google.com
CC: bestswngs@gmail.com
CC: razor@blackwall.org
CC: daniel@iogearbox.net
---
 net/core/dev.h |  8 +++++++-
 net/core/dev.c | 16 +++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/core/dev.h b/net/core/dev.h
index 376bac4a82da..628bdaebf0ca 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -31,9 +31,15 @@ struct napi_struct *
 netdev_napi_by_id_lock(struct net *net, unsigned int napi_id);
 struct net_device *dev_get_by_napi_id(unsigned int napi_id);
 
-struct net_device *__netdev_put_lock(struct net_device *dev, struct net *net);
 struct net_device *netdev_put_lock(struct net_device *dev, struct net *net,
 				   netdevice_tracker *tracker);
+
+static inline struct net_device *
+__netdev_put_lock(struct net_device *dev, struct net *net)
+{
+	return netdev_put_lock(dev, net, NULL);
+}
+
 struct net_device *
 netdev_xa_find_lock(struct net *net, struct net_device *dev,
 		    unsigned long *index);
diff --git a/net/core/dev.c b/net/core/dev.c
index e7bc95cbd1fa..4a82983e8616 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1060,16 +1060,18 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id)
  * This helper is intended for locking net_device after it has been looked up
  * using a lockless lookup helper. Lock prevents the instance from going away.
  */
-struct net_device *__netdev_put_lock(struct net_device *dev, struct net *net)
+struct net_device *
+netdev_put_lock(struct net_device *dev, struct net *net,
+		netdevice_tracker *tracker)
 {
 	netdev_lock(dev);
 	if (dev->reg_state > NETREG_REGISTERED ||
 	    dev->moving_ns || !net_eq(dev_net(dev), net)) {
 		netdev_unlock(dev);
-		dev_put(dev);
+		netdev_put(dev, tracker);
 		return NULL;
 	}
-	dev_put(dev);
+	netdev_put(dev, tracker);
 	return dev;
 }
 
@@ -1121,14 +1123,6 @@ netdev_get_by_index_lock_ops_compat(struct net *net, int ifindex)
 	return __netdev_put_lock_ops_compat(dev, net);
 }
 
-struct net_device *
-netdev_put_lock(struct net_device *dev, struct net *net,
-		netdevice_tracker *tracker)
-{
-	netdev_tracker_free(dev, tracker);
-	return __netdev_put_lock(dev, net);
-}
-
 struct net_device *
 netdev_xa_find_lock(struct net *net, struct net_device *dev,
 		    unsigned long *index)
-- 
2.53.0

Re: [PATCH net-next] net: fix reference tracker mismanagement in netdev_put_lock()

[Daniel Borkmann]

On 4/10/26 5:36 PM, Jakub Kicinski wrote:
> dev_put() releases a reference which didn't have a tracker.
> References without a tracker are accounted in the tracking
> code as "no_tracker". We can't free the tracker and then
> call dev_put(). The references themselves will be fine
> but the tracking code will think it's a double-release:
> 
>    refcount_t: decrement hit 0; leaking memory.
> 
> IOW commit under fixes confused dev_put() (release never tracked
> reference) with __dev_put() (just release the reference, skipping
> the reference tracking infra).
> 
> Since __netdev_put_lock() uses dev_put() we can't feed a previously
> tracked netdev ref into it. Let's flip things around.
> netdev_put(dev, NULL) is the same as dev_put(dev) so make
> netdev_put_lock() the real function and have __netdev_put_lock()
> feed it a NULL tracker for all the cases that were untracked.
> 
> Fixes: d04686d9bc86 ("net: Implement netdev_nl_queue_create_doit")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Thanks!

Acked-by: Daniel Borkmann <daniel@iogearbox.net>