Messages in this thread Patch in this message | | | From | Jann Horn <> | Subject | [PATCH net] net: dev: Detect dev_hold() after netdev_wait_allrefs() | Date | Fri, 28 Jan 2022 02:43:03 +0100 |
| |
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
| |