* [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() @ 2018-05-08 16:21 Kirill Tkhai 2018-05-09 7:18 ` Jason Wang 2018-05-10 19:17 ` David Miller 0 siblings, 2 replies; 5+ messages in thread From: Kirill Tkhai @ 2018-05-08 16:21 UTC (permalink / raw) To: davem, jasowang, edumazet, mst, brouer, peterpenkov96, sd, netdev, ktkhai 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 <ktkhai@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) { + 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) ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() 2018-05-08 16:21 [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() Kirill Tkhai @ 2018-05-09 7:18 ` Jason Wang 2018-05-09 9:00 ` Kirill Tkhai 2018-05-10 19:17 ` David Miller 1 sibling, 1 reply; 5+ messages in thread From: Jason Wang @ 2018-05-09 7:18 UTC (permalink / raw) To: Kirill Tkhai, davem, edumazet, mst, brouer, peterpenkov96, sd, netdev 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 <ktkhai@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) > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() 2018-05-09 7:18 ` Jason Wang @ 2018-05-09 9:00 ` Kirill Tkhai 2018-05-09 13:19 ` Jason Wang 0 siblings, 1 reply; 5+ messages in thread From: Kirill Tkhai @ 2018-05-09 9:00 UTC (permalink / raw) To: Jason Wang, davem, edumazet, mst, brouer, peterpenkov96, sd, netdev Hi, Jason, On 09.05.2018 10:18, Jason Wang wrote: > > > 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 <ktkhai@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. The most of socket ioctl cmds use 0x8900 type: #define SOCK_IOC_TYPE 0x89 while tun cmd is 5400 ('T'). They should not intersect. The only exceptions are #define SIOCINQ FIONREAD #define SIOCOUTQ TIOCOUTQ #define TIOCOUTQ 0x5411 #define FIONREAD 0x541B But they can't intersect even with exceptions, since tun nr starts from 200: #define TUNSETNOCSUM _IOW('T', 200, int) and 200 > 0x1b (==27). I implemented SIOCGSKNS cmd in the same style as older socket cmds were used. I'm not sure, we can remove existing SIOCGIFHWADDR, since they are already used. If we add a warn, which time will we able to remove them? Some old software may use it, and in case of the program isn't developed any more, nobody can fix this warnings, even if he/she sees them.. Kirill > >> + 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) >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() 2018-05-09 9:00 ` Kirill Tkhai @ 2018-05-09 13:19 ` Jason Wang 0 siblings, 0 replies; 5+ messages in thread From: Jason Wang @ 2018-05-09 13:19 UTC (permalink / raw) To: Kirill Tkhai, davem, edumazet, mst, brouer, peterpenkov96, sd, netdev On 2018年05月09日 17:00, Kirill Tkhai wrote: > Hi, Jason, > > On 09.05.2018 10:18, Jason Wang wrote: >> >> 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 <ktkhai@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. > The most of socket ioctl cmds use 0x8900 type: > > #define SOCK_IOC_TYPE 0x89 > > while tun cmd is 5400 ('T'). They should not intersect. > > The only exceptions are > > #define SIOCINQ FIONREAD > #define SIOCOUTQ TIOCOUTQ > > #define TIOCOUTQ 0x5411 > #define FIONREAD 0x541B > > But they can't intersect even with exceptions, since tun nr starts from 200: > > #define TUNSETNOCSUM _IOW('T', 200, int) > > and 200 > 0x1b (==27). > > I implemented SIOCGSKNS cmd in the same style as older socket cmds were used. > I'm not sure, we can remove existing SIOCGIFHWADDR, since they are already used. I think it's too late to remove it. > If we add a warn, which time will we able to remove them? Some old software may > use it, and in case of the program isn't developed any more, nobody can fix this > warnings, even if he/she sees them.. I think this give a chance to push new wrote userspace to use new ioctl cmd. Thanks > > Kirill > >>> + 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) >>> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() 2018-05-08 16:21 [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() Kirill Tkhai 2018-05-09 7:18 ` Jason Wang @ 2018-05-10 19:17 ` David Miller 1 sibling, 0 replies; 5+ messages in thread From: David Miller @ 2018-05-10 19:17 UTC (permalink / raw) To: ktkhai; +Cc: jasowang, edumazet, mst, brouer, peterpenkov96, sd, netdev From: Kirill Tkhai <ktkhai@virtuozzo.com> Date: Tue, 08 May 2018 19:21:34 +0300 > 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 <ktkhai@virtuozzo.com> Applied, thank you. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-05-10 19:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-08 16:21 [PATCH net-next] tun: Do SIOCGSKNS out of rtnl_lock() Kirill Tkhai 2018-05-09 7:18 ` Jason Wang 2018-05-09 9:00 ` Kirill Tkhai 2018-05-09 13:19 ` Jason Wang 2018-05-10 19:17 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).