From 39ec7de7092ba9789f211d112a66bd19d4cb5d36 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 16 Dec 2014 15:04:56 +0200 Subject: [PATCH 1/5] macvtap: fix uninitialized access on TUNSETIFF flags field in ifreq is only 16 bit wide, but we read it as a 32 bit value. If userspace doesn't zero-initialize unused fields, this will lead to failures. Signed-off-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- drivers/net/macvtap.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 60f7ee5fafbe..80fdb82352fa 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -999,7 +999,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, void __user *argp = (void __user *)arg; struct ifreq __user *ifr = argp; unsigned int __user *up = argp; - unsigned int u; + unsigned short u; int __user *sp = argp; int s; int ret; @@ -1014,7 +1014,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, if ((u & ~MACVTAP_FEATURES) != (IFF_NO_PI | IFF_TAP)) ret = -EINVAL; else - q->flags = u; + q->flags = (q->flags & ~MACVTAP_FEATURES) | u; return ret; @@ -1027,8 +1027,9 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, } ret = 0; + u = q->flags; if (copy_to_user(&ifr->ifr_name, vlan->dev->name, IFNAMSIZ) || - put_user(q->flags, &ifr->ifr_flags)) + put_user(u, &ifr->ifr_flags)) ret = -EFAULT; macvtap_put_vlan(vlan); rtnl_unlock(); From 5eea84f478537de769330079dc068414f9d417f4 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 16 Dec 2014 15:05:01 +0200 Subject: [PATCH 2/5] if_tun: add TUNSETVNETLE/TUNGETVNETLE ifreq flags field is only 16 bit wide, so setting IFF_VNET_LE there has no effect: doesn't fit in two bytes. The tests passed apparently because they have an even number of bugs, all cancelling out. Luckily we didn't release a kernel with this flag, so it's not too late to fix this. Add TUNSETVNETLE/TUNGETVNETLE to really achieve the purpose of IFF_VNET_LE. This has an added benefit that if we ever want a BE flag, we won't have to deal with weird configurations like setting both LE and BE at the same time. IFF_VNET_LE will be dropped in a follow-up patch. Reported-by: Dan Carpenter Signed-off-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- include/uapi/linux/if_tun.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h index 18b2403982f9..274630caa276 100644 --- a/include/uapi/linux/if_tun.h +++ b/include/uapi/linux/if_tun.h @@ -48,6 +48,8 @@ #define TUNSETQUEUE _IOW('T', 217, int) #define TUNSETIFINDEX _IOW('T', 218, unsigned int) #define TUNGETFILTER _IOR('T', 219, struct sock_fprog) +#define TUNSETVNETLE _IOW('T', 220, int) +#define TUNGETVNETLE _IOR('T', 221, int) /* TUNSETIFF ifr flags */ #define IFF_TUN 0x0001 From 1cf8e410b6d3906aa1ecacdd15dd9b448c9ce111 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 16 Dec 2014 15:05:06 +0200 Subject: [PATCH 3/5] tun: drop broken IFF_VNET_LE Use TUNSETVNETLE/TUNGETVNETLE instead. Signed-off-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- drivers/net/tun.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index a5cbf67517f0..8c8dc16839a7 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -110,9 +110,11 @@ do { \ * overload it to mean fasync when stored there. */ #define TUN_FASYNC IFF_ATTACH_QUEUE +/* High bits in flags field are unused. */ +#define TUN_VNET_LE 0x80000000 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \ - IFF_VNET_LE | IFF_MULTI_QUEUE) + IFF_MULTI_QUEUE) #define GOODCOPY_LEN 128 #define FLT_EXACT_COUNT 8 @@ -208,12 +210,12 @@ struct tun_struct { static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16 val) { - return __virtio16_to_cpu(tun->flags & IFF_VNET_LE, val); + return __virtio16_to_cpu(tun->flags & TUN_VNET_LE, val); } static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16 val) { - return __cpu_to_virtio16(tun->flags & IFF_VNET_LE, val); + return __cpu_to_virtio16(tun->flags & TUN_VNET_LE, val); } static inline u32 tun_hashfn(u32 rxhash) @@ -1843,6 +1845,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd, int sndbuf; int vnet_hdr_sz; unsigned int ifindex; + int le; int ret; if (cmd == TUNSETIFF || cmd == TUNSETQUEUE || _IOC_TYPE(cmd) == 0x89) { @@ -2042,6 +2045,23 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd, tun->vnet_hdr_sz = vnet_hdr_sz; break; + case TUNGETVNETLE: + le = !!(tun->flags & TUN_VNET_LE); + if (put_user(le, (int __user *)argp)) + ret = -EFAULT; + break; + + case TUNSETVNETLE: + if (get_user(le, (int __user *)argp)) { + ret = -EFAULT; + break; + } + if (le) + tun->flags |= TUN_VNET_LE; + else + tun->flags &= ~TUN_VNET_LE; + break; + case TUNATTACHFILTER: /* Can be set only for TAPs */ ret = -EINVAL; From 01b07fb3508890b637b97f0fe3a4053a6a7f6fc3 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 16 Dec 2014 15:05:10 +0200 Subject: [PATCH 4/5] macvtap: drop broken IFF_VNET_LE Use TUNSETVNETLE/TUNGETVNETLE instead. Signed-off-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- drivers/net/macvtap.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c index 80fdb82352fa..7df221788cd4 100644 --- a/drivers/net/macvtap.c +++ b/drivers/net/macvtap.c @@ -46,16 +46,18 @@ struct macvtap_queue { struct list_head next; }; -#define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_VNET_LE | IFF_MULTI_QUEUE) +#define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_MULTI_QUEUE) + +#define MACVTAP_VNET_LE 0x80000000 static inline u16 macvtap16_to_cpu(struct macvtap_queue *q, __virtio16 val) { - return __virtio16_to_cpu(q->flags & IFF_VNET_LE, val); + return __virtio16_to_cpu(q->flags & MACVTAP_VNET_LE, val); } static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q, u16 val) { - return __cpu_to_virtio16(q->flags & IFF_VNET_LE, val); + return __cpu_to_virtio16(q->flags & MACVTAP_VNET_LE, val); } static struct proto macvtap_proto = { @@ -1070,6 +1072,21 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd, q->vnet_hdr_sz = s; return 0; + case TUNGETVNETLE: + s = !!(q->flags & MACVTAP_VNET_LE); + if (put_user(s, sp)) + return -EFAULT; + return 0; + + case TUNSETVNETLE: + if (get_user(s, sp)) + return -EFAULT; + if (s) + q->flags |= MACVTAP_VNET_LE; + else + q->flags &= ~MACVTAP_VNET_LE; + return 0; + case TUNSETOFFLOAD: /* let the user check for future flags */ if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 | From 9c6ab1931fd6198eab61d9d59aff9a1014637ace Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 16 Dec 2014 15:05:23 +0200 Subject: [PATCH 5/5] if_tun: drop broken IFF_VNET_LE Everyone should use TUNSETVNETLE/TUNGETVNETLE instead. Signed-off-by: Michael S. Tsirkin Signed-off-by: David S. Miller --- include/uapi/linux/if_tun.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h index 274630caa276..50ae24335444 100644 --- a/include/uapi/linux/if_tun.h +++ b/include/uapi/linux/if_tun.h @@ -59,7 +59,6 @@ #define IFF_ONE_QUEUE 0x2000 #define IFF_VNET_HDR 0x4000 #define IFF_TUN_EXCL 0x8000 -#define IFF_VNET_LE 0x10000 #define IFF_MULTI_QUEUE 0x0100 #define IFF_ATTACH_QUEUE 0x0200 #define IFF_DETACH_QUEUE 0x0400