Re: [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock()

[Jason Wang]

On 2018年05月09日 00:21, Kirill Tkhai wrote:
Since net ns of tun device is assigned on the device creation,
and it never changes, we do not need to use any lock to get it
from alive tun.

Signed-off-by: Kirill Tkhai <ktk...@virtuozzo.com>
---
  drivers/net/tun.c |   18 +++++++-----------
  1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d3c04ab9752a..44d4f3d25350 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2850,10 +2850,10 @@ static long __tun_chr_ioctl(struct file *file, unsigned 
int cmd,
                            unsigned long arg, int ifreq_len)
  {
        struct tun_file *tfile = file->private_data;
+       struct net *net = sock_net(&tfile->sk);
        struct tun_struct *tun;
        void __user* argp = (void __user*)arg;
        struct ifreq ifr;
-       struct net *net;
        kuid_t owner;
        kgid_t group;
        int sndbuf;
@@ -2877,14 +2877,18 @@ static long __tun_chr_ioctl(struct file *file, unsigned 
int cmd,
                 */
                return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
                                (unsigned int __user*)argp);
-       } else if (cmd == TUNSETQUEUE)
+       } else if (cmd == TUNSETQUEUE) {
                return tun_set_queue(file, &ifr);
+       } else if (cmd == SIOCGSKNS) {

Not for this patch, reusing socket ioctl cmd is probably not good though 
they were probably not intersected (see ioctl-number.txt). We probably 
need to introduce TUN specific ioctls for SIOCGSKNS and SIOCGIFHWADDR 
and warn for socket ones.

Thanks

+               if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+                       return -EPERM;
+               return open_related_ns(&net->ns, get_net_ns);
+       }
  
  	ret = 0;
        rtnl_lock();
  
  	tun = tun_get(tfile);
-       net = sock_net(&tfile->sk);
        if (cmd == TUNSETIFF) {
                ret = -EEXIST;
                if (tun)
@@ -2914,14 +2918,6 @@ static long __tun_chr_ioctl(struct file *file, unsigned 
int cmd,
                tfile->ifindex = ifindex;
                goto unlock;
        }
-       if (cmd == SIOCGSKNS) {
-               ret = -EPERM;
-               if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
-                       goto unlock;
-
-               ret = open_related_ns(&net->ns, get_net_ns);
-               goto unlock;
-       }
  
  	ret = -EBADFD;
        if (!tun)