lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH net] net: dev: Detect dev_hold() after netdev_wait_allrefs()
Date
I've run into a bug where dev_hold() was being called after
netdev_wait_allrefs(). But at that point, the device is already going
away, and dev_hold() can't stop that anymore.

To make such problems easier to diagnose in the future:

- For CONFIG_PCPU_DEV_REFCNT builds: Recheck in free_netdev() whether
the net refcount has been elevated. If this is detected, WARN() and
leak the object (to prevent worse consequences from a
use-after-free).
- For builds without CONFIG_PCPU_DEV_REFCNT: Set the refcount to zero.
This signals to the generic refcount infrastructure that any attempt
to increment the refcount later is a bug.

Signed-off-by: Jann Horn <jannh@google.com>
---
net/core/dev.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1baab07820f6..f7916c0d226d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9949,8 +9949,18 @@ void netdev_run_todo(void)

netdev_wait_allrefs(dev);

+ /* Drop the netdev refcount (which should be 1 at this point)
+ * to zero. If we're using the generic refcount code, this will
+ * tell it that any dev_hold() after this point is a bug.
+ */
+#ifdef CONFIG_PCPU_DEV_REFCNT
+ this_cpu_dec(*dev->pcpu_refcnt);
+ BUG_ON(netdev_refcnt_read(dev) != 0);
+#else
+ BUG_ON(!refcount_dec_and_test(&dev->dev_refcnt));
+#endif
+
/* paranoia */
- BUG_ON(netdev_refcnt_read(dev) != 1);
BUG_ON(!list_empty(&dev->ptype_all));
BUG_ON(!list_empty(&dev->ptype_specific));
WARN_ON(rcu_access_pointer(dev->ip_ptr));
@@ -10293,6 +10303,12 @@ void free_netdev(struct net_device *dev)
free_percpu(dev->xdp_bulkq);
dev->xdp_bulkq = NULL;

+ /* Recheck in case someone called dev_hold() between
+ * netdev_wait_allrefs() and here.
+ */
+ if (WARN_ON(netdev_refcnt_read(dev) != 0))
+ return; /* leak memory, otherwise we might get UAF */
+
/* Compatibility with error handling in drivers */
if (dev->reg_state == NETREG_UNINITIALIZED) {
netdev_freemem(dev);
base-commit: 23a46422c56144939c091c76cf389aa863ce9c18
--
2.35.0.rc0.227.g00780c9af4-goog

\
 
 \ /
  Last update: 2022-01-28 02:45    [W:0.067 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site