lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141129171718.GA22746@redhat.com>
Date:	Sat, 29 Nov 2014 19:17:18 +0200
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Jason Wang <jasowang@...hat.com>
Cc:	linux-kernel@...r.kernel.org, David Miller <davem@...emloft.net>,
	cornelia.huck@...ibm.com, rusty@....ibm.com, nab@...ux-iscsi.org,
	pbonzini@...hat.com, thuth@...ux.vnet.ibm.com,
	dahi@...ux.vnet.ibm.com, Zhi Yong Wu <wuzhy@...ux.vnet.ibm.com>,
	Ben Hutchings <ben@...adent.org.uk>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Tom Herbert <therbert@...gle.com>,
	Masatake YAMATO <yamato@...hat.com>, Xi Wang <xii@...gle.com>,
	netdev@...r.kernel.org, linux-api@...r.kernel.org
Subject: Re: [PATCH v6 37/46] tun: move internal flag defines out of uapi

On Fri, Nov 28, 2014 at 08:15:32AM +0008, Jason Wang wrote:
> 
> 
> On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst@...hat.com> wrote:
> >TUN_ flags are internal and never exposed
> >to userspace. Any application using it is almost
> >certainly buggy.
> >
> >Move them out to tun.c, we'll remove them in follow-up patches.
> >
> >Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> >---
> > include/uapi/linux/if_tun.h | 16 ++--------
> > drivers/net/tun.c           | 74
> >++++++++++++++-------------------------------
> > 2 files changed, 26 insertions(+), 64 deletions(-)
> >
> >diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> >index e9502dd..277a260 100644
> >--- a/include/uapi/linux/if_tun.h
> >+++ b/include/uapi/linux/if_tun.h
> >@@ -22,21 +22,11 @@
> > /* Read queue size */
> > #define TUN_READQ_SIZE	500
> >-
> >-/* TUN device flags */
> >-#define TUN_TUN_DEV 	0x0001	
> >-#define TUN_TAP_DEV	0x0002
> >+/* TUN device type flags: deprecated. Use IFF_TUN/IFF_TAP instead. */
> >+#define TUN_TUN_DEV 	IFF_TUN
> >+#define TUN_TAP_DEV	IFF_TAP
> > #define TUN_TYPE_MASK   0x000f
> >-#define TUN_FASYNC	0x0010
> >-#define TUN_NOCHECKSUM	0x0020
> >-#define TUN_NO_PI	0x0040
> >-/* This flag has no real effect */
> >-#define TUN_ONE_QUEUE	0x0080
> >-#define TUN_PERSIST 	0x0100	
> >-#define TUN_VNET_HDR 	0x0200
> >-#define TUN_TAP_MQ      0x0400
> >-
> > /* Ioctl defines */
> > #define TUNSETNOCSUM  _IOW('T', 200, int)  #define TUNSETDEBUG
> >_IOW('T', 201, int) diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >index 9dd3746..bc89d07 100644
> >--- a/drivers/net/tun.c
> >+++ b/drivers/net/tun.c
> >@@ -103,6 +103,21 @@ do {								\
> > } while (0)
> > #endif
> >+/* TUN device flags */
> >+
> >+/* IFF_ATTACH_QUEUE is never stored in device flags,
> >+ * overload it to mean fasync when stored there.
> >+ */
> >+#define TUN_FASYNC	IFF_ATTACH_QUEUE
> >+#define TUN_NO_PI	IFF_NO_PI
> >+/* This flag has no real effect */
> >+#define TUN_ONE_QUEUE	IFF_ONE_QUEUE
> >+#define TUN_PERSIST 	IFF_PERSIST
> >+#define TUN_VNET_HDR 	IFF_VNET_HDR
> >+#define TUN_TAP_MQ      IFF_MULTI_QUEUE
> >+
> >+#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> >+		      IFF_MULTI_QUEUE)
> > #define GOODCOPY_LEN 128
> > #define FLT_EXACT_COUNT 8
> >@@ -1521,32 +1536,7 @@ static struct proto tun_proto = {
> > static int tun_flags(struct tun_struct *tun)
> > {
> >-	int flags = 0;
> >-
> >-	if (tun->flags & TUN_TUN_DEV)
> >-		flags |= IFF_TUN;
> >-	else
> >-		flags |= IFF_TAP;
> >-
> >-	if (tun->flags & TUN_NO_PI)
> >-		flags |= IFF_NO_PI;
> >-
> >-	/* This flag has no real effect.  We track the value for backwards
> >-	 * compatibility.
> >-	 */
> >-	if (tun->flags & TUN_ONE_QUEUE)
> >-		flags |= IFF_ONE_QUEUE;
> >-
> >-	if (tun->flags & TUN_VNET_HDR)
> >-		flags |= IFF_VNET_HDR;
> >-
> >-	if (tun->flags & TUN_TAP_MQ)
> >-		flags |= IFF_MULTI_QUEUE;
> >-
> >-	if (tun->flags & TUN_PERSIST)
> >-		flags |= IFF_PERSIST;
> >-
> >-	return flags;
> >+	return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN | IFF_TAP);
> > }
> >   static ssize_t tun_show_flags(struct device *dev, struct
> >device_attribute *attr,
> >@@ -1706,28 +1696,8 @@ static int tun_set_iff(struct net *net, struct file
> >*file, struct ifreq *ifr)
> > 	tun_debug(KERN_INFO, tun, "tun_set_iff\n");
> >-	if (ifr->ifr_flags & IFF_NO_PI)
> >-		tun->flags |= TUN_NO_PI;
> >-	else
> >-		tun->flags &= ~TUN_NO_PI;
> >-
> >-	/* This flag has no real effect.  We track the value for backwards
> >-	 * compatibility.
> >-	 */
> >-	if (ifr->ifr_flags & IFF_ONE_QUEUE)
> >-		tun->flags |= TUN_ONE_QUEUE;
> >-	else
> >-		tun->flags &= ~TUN_ONE_QUEUE;
> >-
> >-	if (ifr->ifr_flags & IFF_VNET_HDR)
> >-		tun->flags |= TUN_VNET_HDR;
> >-	else
> >-		tun->flags &= ~TUN_VNET_HDR;
> >-
> >-	if (ifr->ifr_flags & IFF_MULTI_QUEUE)
> >-		tun->flags |= TUN_TAP_MQ;
> >-	else
> >-		tun->flags &= ~TUN_TAP_MQ;
> >+	tun->flags = (tun->flags & ~TUN_FEATURES) |
> >+		(ifr->ifr_flags & TUN_FEATURES);
> > 	/* Make sure persistent devices do not get stuck in
> > 	 * xoff state.
> >@@ -1890,9 +1860,11 @@ static long __tun_chr_ioctl(struct file *file,
> >unsigned int cmd,
> > 	if (cmd == TUNGETFEATURES) {
> > 		/* Currently this just means: "what IFF flags are valid?".
> > 		 * This is needed because we never checked for invalid flags on
> >-		 * TUNSETIFF. */
> >-		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
> >-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
> >+		 * TUNSETIFF.  Why do we report IFF_TUN and IFF_TAP which are
> >+		 * not legal for TUNSETIFF here?  It's probably a bug, but it
> >+		 * doesn't seem to be worth fixing.
> >+		 */
> 
> The question is not very clear. IFF_TUN and IFF_TAP are legal for
> ifr.ifr_flags during TUNSETIFF. No?


Absolutely. I'm not sure what I meant anymore. I will drop this chunk -
either by replacing this patch if I have to post another version, or
by a separate patch if I don't.

> >
> >+		return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
> > 				(unsigned int __user*)argp);
> > 	} else if (cmd == TUNSETQUEUE)
> > 		return tun_set_queue(file, &ifr);
> >-- 
> >MST
> >
> >--
> >To unsubscribe from this list: send the line "unsubscribe netdev" in
> >the body of a message to majordomo@...r.kernel.org
> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ