From: Jamal Hadi Salim <jhs@mojatatu•com>
To: Cong Wang <xiyou.wangcong@gmail•com>, netdev@vger•kernel.org
Subject: Re: [Patch net] net_sched: close another race condition in tcf_mirred_release()
Date: Tue, 17 May 2016 08:08:02 -0400 [thread overview]
Message-ID: <573B09A2.3040206@mojatatu.com> (raw)
In-Reply-To: <1463436678-20254-1-git-send-email-xiyou.wangcong@gmail.com>
On 16-05-16 06:11 PM, Cong Wang wrote:
> We saw the following extra refcount release on veth device:
>
> kernel: [7957821.463992] unregister_netdevice: waiting for mesos50284 to become free. Usage count = -1
>
> Since we heavily use mirred action to redirect packets to veth, I think
> this is caused by the following race condition:
>
> CPU0:
> tcf_mirred_release(): (in RCU callback)
> struct net_device *dev = rcu_dereference_protected(m->tcfm_dev, 1);
>
> CPU1:
> mirred_device_event():
> spin_lock_bh(&mirred_list_lock);
> list_for_each_entry(m, &mirred_list, tcfm_list) {
> if (rcu_access_pointer(m->tcfm_dev) == dev) {
> dev_put(dev);
> /* Note : no rcu grace period necessary, as
> * net_device are already rcu protected.
> */
> RCU_INIT_POINTER(m->tcfm_dev, NULL);
> }
> }
> spin_unlock_bh(&mirred_list_lock);
>
> CPU0:
> tcf_mirred_release():
> spin_lock_bh(&mirred_list_lock);
> list_del(&m->tcfm_list);
> spin_unlock_bh(&mirred_list_lock);
> if (dev) // <======== Stil refers to the old m->tcfm_dev
> dev_put(dev); // <======== dev_put() is called on it again
>
> The action init code path is good because it is impossible to modify
> an action that is being removed.
>
> So, fix this by moving everything under the spinlock.
>
> Fixes: 2ee22a90c7af ("net_sched: act_mirred: remove spinlock in fast path")
> Fixes: 6bd00b850635 ("act_mirred: fix a race condition on mirred_list")
> Cc: Jamal Hadi Salim <jhs@mojatatu•com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail•com>
LGTM.
Acked-by: Jamal Hadi Salim <jhs@mojatatu•com>
cheers,
jamal
next prev parent reply other threads:[~2016-05-17 12:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-16 22:11 [Patch net] net_sched: close another race condition in tcf_mirred_release() Cong Wang
2016-05-17 12:08 ` Jamal Hadi Salim [this message]
2016-05-17 18:22 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=573B09A2.3040206@mojatatu.com \
--to=jhs@mojatatu$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox