| Message ID | 1357894242-25020-1-git-send-email-jasowang@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Jan 11, 2013 at 04:50:41PM +0800, Jason Wang wrote: > @@ -1546,6 +1544,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) > struct net_device *dev; > int err; > > + if (rcu_dereference_protected(tfile->detached, lockdep_rtnl_is_held())) > + return -EINVAL; > + How come none of the other tfile->detached users call rcu_*()? Stefan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-01-11 at 16:50 +0800, Jason Wang wrote: > > + if (rcu_dereference_protected(tfile->detached, lockdep_rtnl_is_held())) > + return -EINVAL; > + Thats an open coded rtnl_dereference() (please cleanup the whole tun.c file) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/11/2013 10:42 PM, Stefan Hajnoczi wrote: > On Fri, Jan 11, 2013 at 04:50:41PM +0800, Jason Wang wrote: >> @@ -1546,6 +1544,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) >> struct net_device *dev; >> int err; >> >> + if (rcu_dereference_protected(tfile->detached, lockdep_rtnl_is_held())) >> + return -EINVAL; >> + > How come none of the other tfile->detached users call rcu_*()? Right, no need to use rcu, the access were protected by rtnl lock. Will repost. > Stefan > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/11/2013 10:55 PM, Eric Dumazet wrote: > On Fri, 2013-01-11 at 16:50 +0800, Jason Wang wrote: > >> >> + if (rcu_dereference_protected(tfile->detached, lockdep_rtnl_is_held())) >> + return -EINVAL; >> + > Thats an open coded rtnl_dereference() > > (please cleanup the whole tun.c file) > Sure. > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 99b58d8..9a46d70 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -491,8 +491,6 @@ static int tun_attach(struct tun_struct *tun, struct file *file) err = -EINVAL; if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held())) goto out; - if (tfile->detached && tun != tfile->detached) - goto out; err = -EBUSY; if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1) @@ -1546,6 +1544,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr) struct net_device *dev; int err; + if (rcu_dereference_protected(tfile->detached, lockdep_rtnl_is_held())) + return -EINVAL; + dev = __dev_get_by_name(net, ifr->ifr_name); if (dev) { if (ifr->ifr_flags & IFF_TUN_EXCL)
Michael points out that even after Stefan's fix the TUNSETIFF is still allowed to create a new tap device. This because we only check tfile->tun but the tfile->detached were introduced. Fix this by failing early in tun_set_iff() if the file is detached. After this fix, there's no need to do the check again in tun_set_iff(), so this patch removes it. Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/net/tun.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)