| Message ID | 20210322182145.531377-1-eric.dumazet@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] net: set initial device refcount to 1 | expand |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 22 Mar 2021 11:21:45 -0700 > From: Eric Dumazet <edumazet@google.com> > > When adding CONFIG_PCPU_DEV_REFCNT, I forgot that the > initial net device refcount was 0. > > When CONFIG_PCPU_DEV_REFCNT is not set, this means > the first dev_hold() triggers an illegal refcount > operation (addition on 0) > > refcount_t: addition on 0; use-after-free. > WARNING: CPU: 0 PID: 1 at lib/refcount.c:25 refcount_warn_saturate+0x128/0x1a4 > > Fix is to change initial (and final) refcount to be 1. > > Also add a missing kerneldoc piece, as reported by > Stephen Rothwell. > > Fixes: 919067cc845f ("net: add CONFIG_PCPU_DEV_REFCNT") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Guenter Roeck <groeck@google.com> This hunk: > @@ -10682,6 +10682,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, > dev->pcpu_refcnt = alloc_percpu(int); > if (!dev->pcpu_refcnt) > goto free_dev; > + dev_hold(dev); > +#else > + refcount_set(&dev->dev_refcnt, 1); > #endif > > if (dev_addr_init(dev)) gets rejects in net-next. Please respin.
From: David Miller <davem@davemloft.net> Date: Mon, 22 Mar 2021 16:55:26 -0700 (PDT) > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 22 Mar 2021 11:21:45 -0700 > > This hunk: >> @@ -10682,6 +10682,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, >> dev->pcpu_refcnt = alloc_percpu(int); >> if (!dev->pcpu_refcnt) >> goto free_dev; >> + dev_hold(dev); >> +#else >> + refcount_set(&dev->dev_refcnt, 1); >> #endif >> >> if (dev_addr_init(dev)) > gets rejects in net-next. Please respin. > My bad, I was trying to apply it to 'net' mistakedly. All good now, thanks.
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Mon, 22 Mar 2021 11:21:45 -0700 you wrote: > From: Eric Dumazet <edumazet@google.com> > > When adding CONFIG_PCPU_DEV_REFCNT, I forgot that the > initial net device refcount was 0. > > When CONFIG_PCPU_DEV_REFCNT is not set, this means > the first dev_hold() triggers an illegal refcount > operation (addition on 0) > > [...] Here is the summary with links: - [net-next] net: set initial device refcount to 1 https://git.kernel.org/netdev/net-next/c/add2d7363107 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 8f003955c485b81210ed56f7e1c24080b4bb46eb..b11c2c1890b2a28ba2d02fc4466380703a12efaf 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1792,6 +1792,7 @@ enum netdev_ml_priv_type { * * @proto_down_reason: reason a netdev interface is held down * @pcpu_refcnt: Number of references to this device + * @dev_refcnt: Number of references to this device * @todo_list: Delayed register/unregister * @link_watch_list: XXX: need comments on this one * diff --git a/net/core/dev.c b/net/core/dev.c index be941ed754ac71d0839604bcfdd8ab67c339d27f..95c78279d900796c8a3ed0df59b168d5c5e0e309 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -10348,7 +10348,7 @@ static void netdev_wait_allrefs(struct net_device *dev) rebroadcast_time = warning_time = jiffies; refcnt = netdev_refcnt_read(dev); - while (refcnt != 0) { + while (refcnt != 1) { if (time_after(jiffies, rebroadcast_time + 1 * HZ)) { rtnl_lock(); @@ -10385,7 +10385,7 @@ static void netdev_wait_allrefs(struct net_device *dev) refcnt = netdev_refcnt_read(dev); - if (refcnt && time_after(jiffies, warning_time + 10 * HZ)) { + if (refcnt != 1 && time_after(jiffies, warning_time + 10 * HZ)) { pr_emerg("unregister_netdevice: waiting for %s to become free. Usage count = %d\n", dev->name, refcnt); warning_time = jiffies; @@ -10461,7 +10461,7 @@ void netdev_run_todo(void) netdev_wait_allrefs(dev); /* paranoia */ - BUG_ON(netdev_refcnt_read(dev)); + 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)); @@ -10682,6 +10682,9 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name, dev->pcpu_refcnt = alloc_percpu(int); if (!dev->pcpu_refcnt) goto free_dev; + dev_hold(dev); +#else + refcount_set(&dev->dev_refcnt, 1); #endif if (dev_addr_init(dev))