[PATCH 0/3] wireless extensions improvements

[PATCH 0/3] wireless extensions improvements

[Johannes Berg]

Hi,

This series contains three improvements to wireless
extensions:
 1) make them network namespace aware, as discussed
    previously
 2) optimise the event sending code and fix a small
    heap information leak
 3) do some generic work and make it possible to send
    compat netlink events, and make wext do this

Because the third patch depends on the first two (it
could be made not to, if you wish) but touches generic
networking stuff, I'd like to have all of these included
in net-next instead of John's wireless tree, all the
other wireless work can be done independently of these.

johannes

[PATCH 1/3] wireless extensions: make netns aware

[Johannes Berg]

[-- Attachment #1: 003-wext-netns.patch --]
[-- Type: text/plain, Size: 3816 bytes --]

This makes wireless extensions netns aware. The
tasklet sending the events is converted to a work
struct so that we can rtnl_lock() in it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 include/net/net_namespace.h |    3 ++
 net/wireless/wext.c         |   61 ++++++++++++++++++++------------------------
 2 files changed, 32 insertions(+), 32 deletions(-)

--- wireless-testing.orig/include/net/net_namespace.h	2009-06-22 14:44:06.000000000 +0200
+++ wireless-testing/include/net/net_namespace.h	2009-06-22 14:46:22.000000000 +0200
@@ -78,6 +78,9 @@ struct net {
 #ifdef CONFIG_XFRM
 	struct netns_xfrm	xfrm;
 #endif
+#ifdef CONFIG_WIRELESS_EXT
+	struct sk_buff_head	wext_nlevents;
+#endif
 	struct net_generic	*gen;
 };
 
--- wireless-testing.orig/net/wireless/wext.c	2009-06-22 14:44:06.000000000 +0200
+++ wireless-testing/net/wireless/wext.c	2009-06-22 14:46:22.000000000 +0200
@@ -1250,48 +1250,48 @@ int compat_wext_handle_ioctl(struct net 
 }
 #endif
 
-/************************* EVENT PROCESSING *************************/
-/*
- * Process events generated by the wireless layer or the driver.
- * Most often, the event will be propagated through rtnetlink
- */
+static int __net_init wext_pernet_init(struct net *net)
+{
+	skb_queue_head_init(&net->wext_nlevents);
+	return 0;
+}
 
-/* ---------------------------------------------------------------- */
-/*
- * Locking...
- * ----------
- *
- * Thanks to Herbert Xu <herbert@gondor.apana.org.au> for fixing
- * the locking issue in here and implementing this code !
- *
- * The issue : wireless_send_event() is often called in interrupt context,
- * while the Netlink layer can never be called in interrupt context.
- * The fully formed RtNetlink events are queued, and then a tasklet is run
- * to feed those to Netlink.
- * The skb_queue is interrupt safe, and its lock is not held while calling
- * Netlink, so there is no possibility of dealock.
- * Jean II
- */
+static void __net_exit wext_pernet_exit(struct net *net)
+{
+	skb_queue_purge(&net->wext_nlevents);
+}
 
-static struct sk_buff_head wireless_nlevent_queue;
+static struct pernet_operations wext_pernet_ops = {
+	.init = wext_pernet_init,
+	.exit = wext_pernet_exit,
+};
 
 static int __init wireless_nlevent_init(void)
 {
-	skb_queue_head_init(&wireless_nlevent_queue);
+	return register_pernet_subsys(&wext_pernet_ops);
 	return 0;
 }
 
 subsys_initcall(wireless_nlevent_init);
 
-static void wireless_nlevent_process(unsigned long data)
+/* Process events generated by the wireless layer or the driver. */
+static void wireless_nlevent_process(struct work_struct *work)
 {
 	struct sk_buff *skb;
+	struct net *net;
 
-	while ((skb = skb_dequeue(&wireless_nlevent_queue)))
-		rtnl_notify(skb, &init_net, 0, RTNLGRP_LINK, NULL, GFP_ATOMIC);
+	rtnl_lock();
+
+	for_each_net(net) {
+		while ((skb = skb_dequeue(&net->wext_nlevents)))
+			rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL,
+				    GFP_KERNEL);
+	}
+
+	rtnl_unlock();
 }
 
-static DECLARE_TASKLET(wireless_nlevent_tasklet, wireless_nlevent_process, 0);
+static DECLARE_WORK(wireless_nlevent_work, wireless_nlevent_process);
 
 /* ---------------------------------------------------------------- */
 /*
@@ -1341,9 +1341,6 @@ static void rtmsg_iwinfo(struct net_devi
 	struct sk_buff *skb;
 	int err;
 
-	if (!net_eq(dev_net(dev), &init_net))
-		return;
-
 	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
 	if (!skb)
 		return;
@@ -1356,8 +1353,8 @@ static void rtmsg_iwinfo(struct net_devi
 	}
 
 	NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
-	skb_queue_tail(&wireless_nlevent_queue, skb);
-	tasklet_schedule(&wireless_nlevent_tasklet);
+	skb_queue_tail(&dev_net(dev)->wext_nlevents, skb);
+	schedule_work(&wireless_nlevent_work);
 }
 
 /* ---------------------------------------------------------------- */

-- 

[PATCH 2/3] wext: optimise, comment and fix event sending

[Johannes Berg]

[-- Attachment #1: 004-wext-optimise-avoid-leak.patch --]
[-- Type: text/plain, Size: 5614 bytes --]

The current function for sending events first allocates the
event stream buffer, and then an skb to copy the event stream
into. This can be done in one go. Also, the current function
leaks kernel data to userspace in a 4 uninitialised bytes,
initialise those explicitly. Finally also add a few useful
comments, as opposed to the current comments.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 net/wireless/wext.c |  114 ++++++++++++++++++++++++++--------------------------
 1 file changed, 57 insertions(+), 57 deletions(-)

--- wireless-testing.orig/net/wireless/wext.c	2009-06-24 13:31:15.000000000 +0200
+++ wireless-testing/net/wireless/wext.c	2009-06-24 13:31:17.000000000 +0200
@@ -1293,22 +1293,15 @@ static void wireless_nlevent_process(str
 
 static DECLARE_WORK(wireless_nlevent_work, wireless_nlevent_process);
 
-/* ---------------------------------------------------------------- */
-/*
- * Fill a rtnetlink message with our event data.
- * Note that we propage only the specified event and don't dump the
- * current wireless config. Dumping the wireless config is far too
- * expensive (for each parameter, the driver need to query the hardware).
- */
-static int rtnetlink_fill_iwinfo(struct sk_buff *skb, struct net_device *dev,
-				 int type, char *event, int event_len)
+static struct nlmsghdr *rtnetlink_ifinfo_prep(struct net_device *dev,
+					      struct sk_buff *skb)
 {
 	struct ifinfomsg *r;
 	struct nlmsghdr  *nlh;
 
-	nlh = nlmsg_put(skb, 0, 0, type, sizeof(*r), 0);
-	if (nlh == NULL)
-		return -EMSGSIZE;
+	nlh = nlmsg_put(skb, 0, 0, RTM_NEWLINK, sizeof(*r), 0);
+	if (!nlh)
+		return NULL;
 
 	r = nlmsg_data(nlh);
 	r->ifi_family = AF_UNSPEC;
@@ -1319,45 +1312,14 @@ static int rtnetlink_fill_iwinfo(struct 
 	r->ifi_change = 0;	/* Wireless changes don't affect those flags */
 
 	NLA_PUT_STRING(skb, IFLA_IFNAME, dev->name);
-	/* Add the wireless events in the netlink packet */
-	NLA_PUT(skb, IFLA_WIRELESS, event_len, event);
-
-	return nlmsg_end(skb, nlh);
 
-nla_put_failure:
+	return nlh;
+ nla_put_failure:
 	nlmsg_cancel(skb, nlh);
-	return -EMSGSIZE;
+	return NULL;
 }
 
-/* ---------------------------------------------------------------- */
-/*
- * Create and broadcast and send it on the standard rtnetlink socket
- * This is a pure clone rtmsg_ifinfo() in net/core/rtnetlink.c
- * Andrzej Krzysztofowicz mandated that I used a IFLA_XXX field
- * within a RTM_NEWLINK event.
- */
-static void rtmsg_iwinfo(struct net_device *dev, char *event, int event_len)
-{
-	struct sk_buff *skb;
-	int err;
-
-	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
-	if (!skb)
-		return;
-
-	err = rtnetlink_fill_iwinfo(skb, dev, RTM_NEWLINK, event, event_len);
-	if (err < 0) {
-		WARN_ON(err == -EMSGSIZE);
-		kfree_skb(skb);
-		return;
-	}
-
-	NETLINK_CB(skb).dst_group = RTNLGRP_LINK;
-	skb_queue_tail(&dev_net(dev)->wext_nlevents, skb);
-	schedule_work(&wireless_nlevent_work);
-}
 
-/* ---------------------------------------------------------------- */
 /*
  * Main event dispatcher. Called from other parts and drivers.
  * Send the event on the appropriate channels.
@@ -1376,6 +1338,9 @@ void wireless_send_event(struct net_devi
 	int wrqu_off = 0;			/* Offset in wrqu */
 	/* Don't "optimise" the following variable, it will crash */
 	unsigned	cmd_index;		/* *MUST* be unsigned */
+	struct sk_buff *skb;
+	struct nlmsghdr *nlh;
+	struct nlattr *nla;
 
 	/* Get the description of the Event */
 	if (cmd <= SIOCIWLAST) {
@@ -1423,25 +1388,60 @@ void wireless_send_event(struct net_devi
 	hdr_len = event_type_size[descr->header_type];
 	event_len = hdr_len + extra_len;
 
-	/* Create temporary buffer to hold the event */
-	event = kmalloc(event_len, GFP_ATOMIC);
-	if (event == NULL)
+	/*
+	 * The problem for 64/32 bit.
+	 *
+	 * On 64-bit, a regular event is laid out as follows:
+	 *      |  0  |  1  |  2  |  3  |  4  |  5  |  6  |  7  |
+	 *      | event.len | event.cmd |     p a d d i n g     |
+	 *      | wrqu data ... (with the correct size)         |
+	 *
+	 * This padding exists because we manipulate event->u,
+	 * and 'event' is not packed.
+	 *
+	 * An iw_point event is laid out like this instead:
+	 *      |  0  |  1  |  2  |  3  |  4  |  5  |  6  |  7  |
+	 *      | event.len | event.cmd |     p a d d i n g     |
+	 *      | iwpnt.len | iwpnt.flg |     p a d d i n g     |
+	 *      | extra data  ...
+	 *
+	 * The second padding exists because struct iw_point is extended,
+	 * but this depends on the platform...
+	 *
+	 * On 32-bit, all the padding shouldn't be there.
+	 */
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb)
 		return;
 
-	/* Fill event */
+	/* Send via the RtNetlink event channel */
+	nlh = rtnetlink_ifinfo_prep(dev, skb);
+	if (WARN_ON(!nlh)) {
+		kfree_skb(skb);
+		return;
+	}
+
+	/* Add the wireless events in the netlink packet */
+	nla = nla_reserve(skb, IFLA_WIRELESS, event_len);
+	if (!nla) {
+		kfree_skb(skb);
+		return;
+	}
+	event = nla_data(nla);
+
+	/* Fill event - first clear to avoid data leaking */
+	memset(event, 0, hdr_len);
 	event->len = event_len;
 	event->cmd = cmd;
 	memcpy(&event->u, ((char *) wrqu) + wrqu_off, hdr_len - IW_EV_LCP_LEN);
-	if (extra)
+	if (extra_len)
 		memcpy(((char *) event) + hdr_len, extra, extra_len);
 
-	/* Send via the RtNetlink event channel */
-	rtmsg_iwinfo(dev, (char *) event, event_len);
-
-	/* Cleanup */
-	kfree(event);
+	nlmsg_end(skb, nlh);
 
-	return;		/* Always success, I guess ;-) */
+	skb_queue_tail(&dev_net(dev)->wext_nlevents, skb);
+	schedule_work(&wireless_nlevent_work);
 }
 EXPORT_SYMBOL(wireless_send_event);
 

-- 

[PATCH 3/3] net/compat/wext: send different messages to compat tasks

[Johannes Berg]

[-- Attachment #1: 005-wext-event-compat.patch --]
[-- Type: text/plain, Size: 11979 bytes --]

Wireless extensions have the unfortunate problem that events
are multicast netlink messages, and are not independent of
pointer size. Thus, currently 32-bit tasks on 64-bit platforms
cannot properly receive events and fail with all kinds of
strange problems, for instance wpa_supplicant never notices
disassociations, due to the way the 64-bit event looks (to a
32-bit process), the fact that the address is all zeroes is
lost, it thinks instead it is 00:00:00:00:01:00.

The same problem existed with the ioctls, until David Miller
fixed those some time ago in an heroic effort.

A different problem caused by this is that we cannot send the
ASSOCREQIE/ASSOCRESPIE events because sending them causes a
32-bit wpa_supplicant on a 64-bit system to overwrite its
internal information, which is worse than it not getting the
information at all -- so we currently resort to sending a
custom string event that it then parses. This, however, has a
severe size limitation we are frequently hitting with modern
access points; this limitation would can be lifted after this
patch by sending the correct binary, not custom, event.

A similar problem apparently happens for some other netlink
users on x86_64 with 32-bit tasks due to the alignment for
64-bit quantities.

In order to fix these problems, I have implemented a way to
send compat messages to tasks. When sending an event, we send
the non-compat event data together with a compat event data in
skb_shinfo(main_skb)->frag_list. Then, when the event is read
from the socket, the netlink code makes sure to pass out only
the skb that is compatible with the task. This approach was
suggested by David Miller, my original approach required
always sending two skbs but that had various small problems.

To determine whether compat is needed or not, I have used the
MSG_CMSG_COMPAT flag, and adjusted the call path for recv and
recvfrom to include it, even if those calls do not have a cmsg
parameter.

I have not solved one small part of the problem, and I don't
think it is necessary to: if a 32-bit application uses read()
rather than any form of recvmsg() it will still get the wrong
(64-bit) event. However, neither do applications actually do
this, nor would it be a regression.

Signed-off-by: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
---
v2: * after some discussion with davem, use frag_list as noted,
      and note that there are more possible uses for this
    * add another Kconfig symbol for code that can use select
      for the feature
    * add help, in Kconfig, describing the feature

 arch/mips/kernel/scall64-n32.S |    2 -
 arch/mips/kernel/scall64-o32.S |    4 +-
 arch/sparc/kernel/sys32.S      |    2 -
 include/linux/wireless.h       |    8 ++++
 net/Kconfig                    |   20 ++++++++++
 net/compat.c                   |   17 +++++++-
 net/netlink/af_netlink.c       |   21 +++++++++++
 net/wireless/wext.c            |   78 +++++++++++++++++++++++++++++++++++++++++
 8 files changed, 146 insertions(+), 6 deletions(-)

--- wireless-testing.orig/net/wireless/wext.c	2009-06-24 13:31:59.000000000 +0200
+++ wireless-testing/net/wireless/wext.c	2009-06-24 13:32:02.000000000 +0200
@@ -417,6 +417,21 @@ static const int event_type_size[] = {
 	IW_EV_QUAL_LEN,			/* IW_HEADER_TYPE_QUAL */
 };
 
+#ifdef CONFIG_COMPAT
+static const int compat_event_type_size[] = {
+	IW_EV_COMPAT_LCP_LEN,		/* IW_HEADER_TYPE_NULL */
+	0,
+	IW_EV_COMPAT_CHAR_LEN,		/* IW_HEADER_TYPE_CHAR */
+	0,
+	IW_EV_COMPAT_UINT_LEN,		/* IW_HEADER_TYPE_UINT */
+	IW_EV_COMPAT_FREQ_LEN,		/* IW_HEADER_TYPE_FREQ */
+	IW_EV_COMPAT_ADDR_LEN,		/* IW_HEADER_TYPE_ADDR */
+	0,
+	IW_EV_COMPAT_POINT_LEN,		/* Without variable payload */
+	IW_EV_COMPAT_PARAM_LEN,		/* IW_HEADER_TYPE_PARAM */
+	IW_EV_COMPAT_QUAL_LEN,		/* IW_HEADER_TYPE_QUAL */
+};
+#endif
 
 /************************ COMMON SUBROUTINES ************************/
 /*
@@ -1341,6 +1356,22 @@ void wireless_send_event(struct net_devi
 	struct sk_buff *skb;
 	struct nlmsghdr *nlh;
 	struct nlattr *nla;
+#ifdef CONFIG_COMPAT
+	struct __compat_iw_event *compat_event;
+	struct compat_iw_point compat_wrqu;
+	struct sk_buff *compskb;
+#endif
+
+	/*
+	 * Nothing in the kernel sends scan events with data, be safe.
+	 * This is necessary because we cannot fix up scan event data
+	 * for compat, due to being contained in 'extra', but normally
+	 * applications are required to retrieve the scan data anyway
+	 * and no data is included in the event, this codifies that
+	 * practice.
+	 */
+	if (WARN_ON(cmd == SIOCGIWSCAN && extra))
+		extra = NULL;
 
 	/* Get the description of the Event */
 	if (cmd <= SIOCIWLAST) {
@@ -1439,7 +1470,54 @@ void wireless_send_event(struct net_devi
 		memcpy(((char *) event) + hdr_len, extra, extra_len);
 
 	nlmsg_end(skb, nlh);
+#ifdef CONFIG_COMPAT
+	hdr_len = compat_event_type_size[descr->header_type];
+	event_len = hdr_len + extra_len;
+
+	compskb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!compskb) {
+		kfree_skb(skb);
+		return;
+	}
+
+	/* Send via the RtNetlink event channel */
+	nlh = rtnetlink_ifinfo_prep(dev, compskb);
+	if (WARN_ON(!nlh)) {
+		kfree_skb(skb);
+		kfree_skb(compskb);
+		return;
+	}
+
+	/* Add the wireless events in the netlink packet */
+	nla = nla_reserve(compskb, IFLA_WIRELESS, event_len);
+	if (!nla) {
+		kfree_skb(skb);
+		kfree_skb(compskb);
+		return;
+	}
+	compat_event = nla_data(nla);
+
+	compat_event->len = event_len;
+	compat_event->cmd = cmd;
+	if (descr->header_type == IW_HEADER_TYPE_POINT) {
+		compat_wrqu.length = wrqu->data.length;
+		compat_wrqu.flags = wrqu->data.flags;
+		memcpy(&compat_event->pointer,
+			((char *) &compat_wrqu) + IW_EV_COMPAT_POINT_OFF,
+			hdr_len - IW_EV_COMPAT_LCP_LEN);
+		if (extra_len)
+			memcpy(((char *) compat_event) + hdr_len,
+				extra, extra_len);
+	} else {
+		/* extra_len must be zero, so no if (extra) needed */
+		memcpy(&compat_event->pointer, wrqu,
+			hdr_len - IW_EV_COMPAT_LCP_LEN);
+	}
 
+	nlmsg_end(compskb, nlh);
+
+	skb_shinfo(skb)->frag_list = compskb;
+#endif
 	skb_queue_tail(&dev_net(dev)->wext_nlevents, skb);
 	schedule_work(&wireless_nlevent_work);
 }
--- wireless-testing.orig/net/Kconfig	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/net/Kconfig	2009-06-24 13:32:02.000000000 +0200
@@ -23,6 +23,26 @@ menuconfig NET
 
 if NET
 
+config WANT_COMPAT_NETLINK_MESSAGES
+	bool
+	help
+	  This option can be selected by other options that need compat
+	  netlink messages.
+
+config COMPAT_NETLINK_MESSAGES
+	def_bool y
+	depends on COMPAT
+	depends on WIRELESS_EXT || WANT_COMPAT_NETLINK_MESSAGES
+	help
+	  This option makes it possible to send different netlink messages
+	  to tasks depending on whether the task is a compat task or not. To
+	  achieve this, you need to set skb_shinfo(skb)->frag_list to the
+	  compat skb before sending the skb, the netlink code will sort out
+	  which message to actually pass to the task.
+
+	  Newly written code should NEVER need this option but do
+	  compat-independent messages instead!
+
 menu "Networking options"
 
 source "net/packet/Kconfig"
--- wireless-testing.orig/net/netlink/af_netlink.c	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/net/netlink/af_netlink.c	2009-06-24 13:32:02.000000000 +0200
@@ -1368,6 +1368,27 @@ static int netlink_recvmsg(struct kiocb 
 	if (skb == NULL)
 		goto out;
 
+#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
+	if (unlikely(skb_shinfo(skb)->frag_list)) {
+		bool need_compat = !!(flags & MSG_CMSG_COMPAT);
+
+		/*
+		 * If this skb has a frag_list, then here that means that
+		 * we will have to use the frag_list skb for compat tasks
+		 * and the regular skb for non-compat tasks.
+		 */
+		if (need_compat) {
+			struct sk_buff *compskb = skb_shinfo(skb)->frag_list;
+			skb_shinfo(skb)->frag_list = NULL;
+			kfree_skb(skb);
+			skb = compskb;
+		} else {
+			kfree_skb(skb_shinfo(skb)->frag_list);
+			skb_shinfo(skb)->frag_list = NULL;
+		}
+	}
+#endif
+
 	msg->msg_namelen = 0;
 
 	copied = skb->len;
--- wireless-testing.orig/net/compat.c	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/net/compat.c	2009-06-24 13:32:02.000000000 +0200
@@ -743,6 +743,18 @@ asmlinkage long compat_sys_recvmsg(int f
 	return sys_recvmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
 }
 
+asmlinkage long compat_sys_recv(int fd, void __user *buf, size_t len, unsigned flags)
+{
+	return sys_recv(fd, buf, len, flags | MSG_CMSG_COMPAT);
+}
+
+asmlinkage long compat_sys_recvfrom(int fd, void __user *buf, size_t len,
+				    unsigned flags, struct sockaddr __user *addr,
+				    int __user *addrlen)
+{
+	return sys_recvfrom(fd, buf, len, flags | MSG_CMSG_COMPAT, addr, addrlen);
+}
+
 asmlinkage long compat_sys_socketcall(int call, u32 __user *args)
 {
 	int ret;
@@ -788,10 +800,11 @@ asmlinkage long compat_sys_socketcall(in
 		ret = sys_sendto(a0, compat_ptr(a1), a[2], a[3], compat_ptr(a[4]), a[5]);
 		break;
 	case SYS_RECV:
-		ret = sys_recv(a0, compat_ptr(a1), a[2], a[3]);
+		ret = compat_sys_recv(a0, compat_ptr(a1), a[2], a[3]);
 		break;
 	case SYS_RECVFROM:
-		ret = sys_recvfrom(a0, compat_ptr(a1), a[2], a[3], compat_ptr(a[4]), compat_ptr(a[5]));
+		ret = compat_sys_recvfrom(a0, compat_ptr(a1), a[2], a[3],
+					  compat_ptr(a[4]), compat_ptr(a[5]));
 		break;
 	case SYS_SHUTDOWN:
 		ret = sys_shutdown(a0,a1);
--- wireless-testing.orig/include/linux/wireless.h	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/include/linux/wireless.h	2009-06-24 13:32:02.000000000 +0200
@@ -1132,6 +1132,14 @@ struct __compat_iw_event {
 };
 #define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer)
 #define IW_EV_COMPAT_POINT_OFF offsetof(struct compat_iw_point, length)
+
+/* Size of the various events for compat */
+#define IW_EV_COMPAT_CHAR_LEN	(IW_EV_COMPAT_LCP_LEN + IFNAMSIZ)
+#define IW_EV_COMPAT_UINT_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(__u32))
+#define IW_EV_COMPAT_FREQ_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_freq))
+#define IW_EV_COMPAT_PARAM_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_param))
+#define IW_EV_COMPAT_ADDR_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct sockaddr))
+#define IW_EV_COMPAT_QUAL_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_quality))
 #define IW_EV_COMPAT_POINT_LEN	\
 	(IW_EV_COMPAT_LCP_LEN + sizeof(struct compat_iw_point) - \
 	 IW_EV_COMPAT_POINT_OFF)
--- wireless-testing.orig/arch/sparc/kernel/sys32.S	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/arch/sparc/kernel/sys32.S	2009-06-24 13:32:02.000000000 +0200
@@ -121,7 +121,7 @@ SIGN2(sys32_syslog, sys_syslog, %o0, %o2
 SIGN1(sys32_umask, sys_umask, %o0)
 SIGN3(sys32_tgkill, sys_tgkill, %o0, %o1, %o2)
 SIGN1(sys32_sendto, sys_sendto, %o0)
-SIGN1(sys32_recvfrom, sys_recvfrom, %o0)
+SIGN1(sys32_recvfrom, compat_sys_recvfrom, %o0)
 SIGN3(sys32_socket, sys_socket, %o0, %o1, %o2)
 SIGN2(sys32_connect, sys_connect, %o0, %o2)
 SIGN2(sys32_bind, sys_bind, %o0, %o2)
--- wireless-testing.orig/arch/mips/kernel/scall64-n32.S	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/arch/mips/kernel/scall64-n32.S	2009-06-24 13:32:02.000000000 +0200
@@ -164,7 +164,7 @@ EXPORT(sysn32_call_table)
 	PTR	sys_connect
 	PTR	sys_accept
 	PTR	sys_sendto
-	PTR	sys_recvfrom
+	PTR	compat_sys_recvfrom
 	PTR	compat_sys_sendmsg		/* 6045 */
 	PTR	compat_sys_recvmsg
 	PTR	sys_shutdown
--- wireless-testing.orig/arch/mips/kernel/scall64-o32.S	2009-06-24 13:30:31.000000000 +0200
+++ wireless-testing/arch/mips/kernel/scall64-o32.S	2009-06-24 13:32:02.000000000 +0200
@@ -378,8 +378,8 @@ sys_call_table:
 	PTR	sys_getsockname
 	PTR	sys_getsockopt
 	PTR	sys_listen
-	PTR	sys_recv			/* 4175 */
-	PTR	sys_recvfrom
+	PTR	compat_sys_recv			/* 4175 */
+	PTR	compat_sys_recvfrom
 	PTR	compat_sys_recvmsg
 	PTR	sys_send
 	PTR	compat_sys_sendmsg

-- 

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3 v3] net/compat/wext: send different messages to compat tasks

[Johannes Berg]

Wireless extensions have the unfortunate problem that events
are multicast netlink messages, and are not independent of
pointer size. Thus, currently 32-bit tasks on 64-bit platforms
cannot properly receive events and fail with all kinds of
strange problems, for instance wpa_supplicant never notices
disassociations, due to the way the 64-bit event looks (to a
32-bit process), the fact that the address is all zeroes is
lost, it thinks instead it is 00:00:00:00:01:00.

The same problem existed with the ioctls, until David Miller
fixed those some time ago in an heroic effort.

A different problem caused by this is that we cannot send the
ASSOCREQIE/ASSOCRESPIE events because sending them causes a
32-bit wpa_supplicant on a 64-bit system to overwrite its
internal information, which is worse than it not getting the
information at all -- so we currently resort to sending a
custom string event that it then parses. This, however, has a
severe size limitation we are frequently hitting with modern
access points; this limitation would can be lifted after this
patch by sending the correct binary, not custom, event.

A similar problem apparently happens for some other netlink
users on x86_64 with 32-bit tasks due to the alignment for
64-bit quantities.

In order to fix these problems, I have implemented a way to
send compat messages to tasks. When sending an event, we send
the non-compat event data together with a compat event data in
skb_shinfo(main_skb)->frag_list. Then, when the event is read
from the socket, the netlink code makes sure to pass out only
the skb that is compatible with the task. This approach was
suggested by David Miller, my original approach required
always sending two skbs but that had various small problems.

To determine whether compat is needed or not, I have used the
MSG_CMSG_COMPAT flag, and adjusted the call path for recv and
recvfrom to include it, even if those calls do not have a cmsg
parameter.

I have not solved one small part of the problem, and I don't
think it is necessary to: if a 32-bit application uses read()
rather than any form of recvmsg() it will still get the wrong
(64-bit) event. However, neither do applications actually do
this, nor would it be a regression.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
v2: * after some discussion with davem, use frag_list as noted,
      and note that there are more possible uses for this
    * add another Kconfig symbol for code that can use select
      for the feature
    * add help, in Kconfig, describing the feature
v3: * don't NULL out frag_list incorrectly, keep as required

 arch/mips/kernel/scall64-n32.S |    2 -
 arch/mips/kernel/scall64-o32.S |    4 +-
 arch/sparc/kernel/sys32.S      |    2 -
 include/linux/wireless.h       |    8 ++++
 net/Kconfig                    |   20 ++++++++++
 net/compat.c                   |   17 +++++++-
 net/netlink/af_netlink.c       |   36 ++++++++++++++++++
 net/wireless/wext.c            |   78 +++++++++++++++++++++++++++++++++++++++++
 8 files changed, 160 insertions(+), 7 deletions(-)

--- wireless-testing.orig/net/wireless/wext.c	2009-07-01 21:46:44.000000000 +0200
+++ wireless-testing/net/wireless/wext.c	2009-07-01 21:46:47.000000000 +0200
@@ -417,6 +417,21 @@ static const int event_type_size[] = {
 	IW_EV_QUAL_LEN,			/* IW_HEADER_TYPE_QUAL */
 };
 
+#ifdef CONFIG_COMPAT
+static const int compat_event_type_size[] = {
+	IW_EV_COMPAT_LCP_LEN,		/* IW_HEADER_TYPE_NULL */
+	0,
+	IW_EV_COMPAT_CHAR_LEN,		/* IW_HEADER_TYPE_CHAR */
+	0,
+	IW_EV_COMPAT_UINT_LEN,		/* IW_HEADER_TYPE_UINT */
+	IW_EV_COMPAT_FREQ_LEN,		/* IW_HEADER_TYPE_FREQ */
+	IW_EV_COMPAT_ADDR_LEN,		/* IW_HEADER_TYPE_ADDR */
+	0,
+	IW_EV_COMPAT_POINT_LEN,		/* Without variable payload */
+	IW_EV_COMPAT_PARAM_LEN,		/* IW_HEADER_TYPE_PARAM */
+	IW_EV_COMPAT_QUAL_LEN,		/* IW_HEADER_TYPE_QUAL */
+};
+#endif
 
 /************************ COMMON SUBROUTINES ************************/
 /*
@@ -1348,6 +1363,22 @@ void wireless_send_event(struct net_devi
 	struct sk_buff *skb;
 	struct nlmsghdr *nlh;
 	struct nlattr *nla;
+#ifdef CONFIG_COMPAT
+	struct __compat_iw_event *compat_event;
+	struct compat_iw_point compat_wrqu;
+	struct sk_buff *compskb;
+#endif
+
+	/*
+	 * Nothing in the kernel sends scan events with data, be safe.
+	 * This is necessary because we cannot fix up scan event data
+	 * for compat, due to being contained in 'extra', but normally
+	 * applications are required to retrieve the scan data anyway
+	 * and no data is included in the event, this codifies that
+	 * practice.
+	 */
+	if (WARN_ON(cmd == SIOCGIWSCAN && extra))
+		extra = NULL;
 
 	/* Get the description of the Event */
 	if (cmd <= SIOCIWLAST) {
@@ -1446,7 +1477,54 @@ void wireless_send_event(struct net_devi
 		memcpy(((char *) event) + hdr_len, extra, extra_len);
 
 	nlmsg_end(skb, nlh);
+#ifdef CONFIG_COMPAT
+	hdr_len = compat_event_type_size[descr->header_type];
+	event_len = hdr_len + extra_len;
+
+	compskb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!compskb) {
+		kfree_skb(skb);
+		return;
+	}
+
+	/* Send via the RtNetlink event channel */
+	nlh = rtnetlink_ifinfo_prep(dev, compskb);
+	if (WARN_ON(!nlh)) {
+		kfree_skb(skb);
+		kfree_skb(compskb);
+		return;
+	}
+
+	/* Add the wireless events in the netlink packet */
+	nla = nla_reserve(compskb, IFLA_WIRELESS, event_len);
+	if (!nla) {
+		kfree_skb(skb);
+		kfree_skb(compskb);
+		return;
+	}
+	compat_event = nla_data(nla);
+
+	compat_event->len = event_len;
+	compat_event->cmd = cmd;
+	if (descr->header_type == IW_HEADER_TYPE_POINT) {
+		compat_wrqu.length = wrqu->data.length;
+		compat_wrqu.flags = wrqu->data.flags;
+		memcpy(&compat_event->pointer,
+			((char *) &compat_wrqu) + IW_EV_COMPAT_POINT_OFF,
+			hdr_len - IW_EV_COMPAT_LCP_LEN);
+		if (extra_len)
+			memcpy(((char *) compat_event) + hdr_len,
+				extra, extra_len);
+	} else {
+		/* extra_len must be zero, so no if (extra) needed */
+		memcpy(&compat_event->pointer, wrqu,
+			hdr_len - IW_EV_COMPAT_LCP_LEN);
+	}
 
+	nlmsg_end(compskb, nlh);
+
+	skb_shinfo(skb)->frag_list = compskb;
+#endif
 	skb_queue_tail(&dev_net(dev)->wext_nlevents, skb);
 	schedule_work(&wireless_nlevent_work);
 }
--- wireless-testing.orig/net/Kconfig	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/net/Kconfig	2009-07-01 21:46:47.000000000 +0200
@@ -23,6 +23,26 @@ menuconfig NET
 
 if NET
 
+config WANT_COMPAT_NETLINK_MESSAGES
+	bool
+	help
+	  This option can be selected by other options that need compat
+	  netlink messages.
+
+config COMPAT_NETLINK_MESSAGES
+	def_bool y
+	depends on COMPAT
+	depends on WIRELESS_EXT || WANT_COMPAT_NETLINK_MESSAGES
+	help
+	  This option makes it possible to send different netlink messages
+	  to tasks depending on whether the task is a compat task or not. To
+	  achieve this, you need to set skb_shinfo(skb)->frag_list to the
+	  compat skb before sending the skb, the netlink code will sort out
+	  which message to actually pass to the task.
+
+	  Newly written code should NEVER need this option but do
+	  compat-independent messages instead!
+
 menu "Networking options"
 
 source "net/packet/Kconfig"
--- wireless-testing.orig/net/netlink/af_netlink.c	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/net/netlink/af_netlink.c	2009-07-01 23:24:21.000000000 +0200
@@ -1356,7 +1356,7 @@ static int netlink_recvmsg(struct kiocb 
 	struct netlink_sock *nlk = nlk_sk(sk);
 	int noblock = flags&MSG_DONTWAIT;
 	size_t copied;
-	struct sk_buff *skb;
+	struct sk_buff *skb, *frag __maybe_unused = NULL;
 	int err;
 
 	if (flags&MSG_OOB)
@@ -1368,6 +1368,35 @@ static int netlink_recvmsg(struct kiocb 
 	if (skb == NULL)
 		goto out;
 
+#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
+	if (unlikely(skb_shinfo(skb)->frag_list)) {
+		bool need_compat = !!(flags & MSG_CMSG_COMPAT);
+
+		/*
+		 * If this skb has a frag_list, then here that means that
+		 * we will have to use the frag_list skb for compat tasks
+		 * and the regular skb for non-compat tasks.
+		 *
+		 * The skb might (and likely will) be cloned, so we can't
+		 * just reset frag_list and go on with things -- we need to
+		 * keep that. For the compat case that's easy -- simply get
+		 * a reference to the compat skb and free the regular one
+		 * including the frag. For the non-compat case, we need to
+		 * avoid sending the frag to the user -- so assign NULL but
+		 * restore it below before freeing the skb.
+		 */
+		if (need_compat) {
+			struct sk_buff *compskb = skb_shinfo(skb)->frag_list;
+			skb_get(compskb);
+			kfree_skb(skb);
+			skb = compskb;
+		} else {
+			frag = skb_shinfo(skb)->frag_list;
+			skb_shinfo(skb)->frag_list = NULL;
+		}
+	}
+#endif
+
 	msg->msg_namelen = 0;
 
 	copied = skb->len;
@@ -1398,6 +1427,11 @@ static int netlink_recvmsg(struct kiocb 
 	siocb->scm->creds = *NETLINK_CREDS(skb);
 	if (flags & MSG_TRUNC)
 		copied = skb->len;
+
+#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
+	skb_shinfo(skb)->frag_list = frag;
+#endif
+
 	skb_free_datagram(sk, skb);
 
 	if (nlk->cb && atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf / 2)
--- wireless-testing.orig/net/compat.c	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/net/compat.c	2009-07-01 21:46:47.000000000 +0200
@@ -743,6 +743,18 @@ asmlinkage long compat_sys_recvmsg(int f
 	return sys_recvmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
 }
 
+asmlinkage long compat_sys_recv(int fd, void __user *buf, size_t len, unsigned flags)
+{
+	return sys_recv(fd, buf, len, flags | MSG_CMSG_COMPAT);
+}
+
+asmlinkage long compat_sys_recvfrom(int fd, void __user *buf, size_t len,
+				    unsigned flags, struct sockaddr __user *addr,
+				    int __user *addrlen)
+{
+	return sys_recvfrom(fd, buf, len, flags | MSG_CMSG_COMPAT, addr, addrlen);
+}
+
 asmlinkage long compat_sys_socketcall(int call, u32 __user *args)
 {
 	int ret;
@@ -788,10 +800,11 @@ asmlinkage long compat_sys_socketcall(in
 		ret = sys_sendto(a0, compat_ptr(a1), a[2], a[3], compat_ptr(a[4]), a[5]);
 		break;
 	case SYS_RECV:
-		ret = sys_recv(a0, compat_ptr(a1), a[2], a[3]);
+		ret = compat_sys_recv(a0, compat_ptr(a1), a[2], a[3]);
 		break;
 	case SYS_RECVFROM:
-		ret = sys_recvfrom(a0, compat_ptr(a1), a[2], a[3], compat_ptr(a[4]), compat_ptr(a[5]));
+		ret = compat_sys_recvfrom(a0, compat_ptr(a1), a[2], a[3],
+					  compat_ptr(a[4]), compat_ptr(a[5]));
 		break;
 	case SYS_SHUTDOWN:
 		ret = sys_shutdown(a0,a1);
--- wireless-testing.orig/include/linux/wireless.h	2009-07-01 21:24:32.000000000 +0200
+++ wireless-testing/include/linux/wireless.h	2009-07-01 21:46:47.000000000 +0200
@@ -1132,6 +1132,14 @@ struct __compat_iw_event {
 };
 #define IW_EV_COMPAT_LCP_LEN offsetof(struct __compat_iw_event, pointer)
 #define IW_EV_COMPAT_POINT_OFF offsetof(struct compat_iw_point, length)
+
+/* Size of the various events for compat */
+#define IW_EV_COMPAT_CHAR_LEN	(IW_EV_COMPAT_LCP_LEN + IFNAMSIZ)
+#define IW_EV_COMPAT_UINT_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(__u32))
+#define IW_EV_COMPAT_FREQ_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_freq))
+#define IW_EV_COMPAT_PARAM_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_param))
+#define IW_EV_COMPAT_ADDR_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct sockaddr))
+#define IW_EV_COMPAT_QUAL_LEN	(IW_EV_COMPAT_LCP_LEN + sizeof(struct iw_quality))
 #define IW_EV_COMPAT_POINT_LEN	\
 	(IW_EV_COMPAT_LCP_LEN + sizeof(struct compat_iw_point) - \
 	 IW_EV_COMPAT_POINT_OFF)
--- wireless-testing.orig/arch/sparc/kernel/sys32.S	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/arch/sparc/kernel/sys32.S	2009-07-01 21:46:48.000000000 +0200
@@ -121,7 +121,7 @@ SIGN2(sys32_syslog, sys_syslog, %o0, %o2
 SIGN1(sys32_umask, sys_umask, %o0)
 SIGN3(sys32_tgkill, sys_tgkill, %o0, %o1, %o2)
 SIGN1(sys32_sendto, sys_sendto, %o0)
-SIGN1(sys32_recvfrom, sys_recvfrom, %o0)
+SIGN1(sys32_recvfrom, compat_sys_recvfrom, %o0)
 SIGN3(sys32_socket, sys_socket, %o0, %o1, %o2)
 SIGN2(sys32_connect, sys_connect, %o0, %o2)
 SIGN2(sys32_bind, sys_bind, %o0, %o2)
--- wireless-testing.orig/arch/mips/kernel/scall64-n32.S	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/arch/mips/kernel/scall64-n32.S	2009-07-01 21:46:48.000000000 +0200
@@ -164,7 +164,7 @@ EXPORT(sysn32_call_table)
 	PTR	sys_connect
 	PTR	sys_accept
 	PTR	sys_sendto
-	PTR	sys_recvfrom
+	PTR	compat_sys_recvfrom
 	PTR	compat_sys_sendmsg		/* 6045 */
 	PTR	compat_sys_recvmsg
 	PTR	sys_shutdown
--- wireless-testing.orig/arch/mips/kernel/scall64-o32.S	2009-07-01 21:24:31.000000000 +0200
+++ wireless-testing/arch/mips/kernel/scall64-o32.S	2009-07-01 21:46:48.000000000 +0200
@@ -378,8 +378,8 @@ sys_call_table:
 	PTR	sys_getsockname
 	PTR	sys_getsockopt
 	PTR	sys_listen
-	PTR	sys_recv			/* 4175 */
-	PTR	sys_recvfrom
+	PTR	compat_sys_recv			/* 4175 */
+	PTR	compat_sys_recvfrom
 	PTR	compat_sys_recvmsg
 	PTR	sys_send
 	PTR	compat_sys_sendmsg

Re: [PATCH 3/3 v3] net/compat/wext: send different messages to compat tasks

[David Miller]

Ok, all 3 patches applied to net-next-2.6, let's see what explodes ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] wireless extensions improvements

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Wed, 24 Jun 2009 13:34:47 +0200

> This series contains three improvements to wireless
> extensions:
>  1) make them network namespace aware, as discussed
>     previously
>  2) optimise the event sending code and fix a small
>     heap information leak
>  3) do some generic work and make it possible to send
>     compat netlink events, and make wext do this
> 
> Because the third patch depends on the first two (it
> could be made not to, if you wish) but touches generic
> networking stuff, I'd like to have all of these included
> in net-next instead of John's wireless tree, all the
> other wireless work can be done independently of these.

These changes look fine to me, so I'll likely put them into
net-next-2.6 as soon as I open that up.

FWIW, I bet we could make read() easily work properly to for the
compat stuff, and it would be nice to shore up that one minor hole.

Re: [PATCH 0/3] wireless extensions improvements

[John W. Linville]

On Thu, Jun 25, 2009 at 04:18:17PM -0700, David Miller wrote:
> From: Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
> Date: Wed, 24 Jun 2009 13:34:47 +0200
> 
> > This series contains three improvements to wireless
> > extensions:
> >  1) make them network namespace aware, as discussed
> >     previously
> >  2) optimise the event sending code and fix a small
> >     heap information leak
> >  3) do some generic work and make it possible to send
> >     compat netlink events, and make wext do this
> > 
> > Because the third patch depends on the first two (it
> > could be made not to, if you wish) but touches generic
> > networking stuff, I'd like to have all of these included
> > in net-next instead of John's wireless tree, all the
> > other wireless work can be done independently of these.
> 
> These changes look fine to me, so I'll likely put them into
> net-next-2.6 as soon as I open that up.

ACK

-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] wireless extensions improvements

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

On Thu, 2009-06-25 at 16:18 -0700, David Miller wrote:

> These changes look fine to me, so I'll likely put them into
> net-next-2.6 as soon as I open that up.

Alright, sounds good.

> FWIW, I bet we could make read() easily work properly to for the
> compat stuff, and it would be nice to shore up that one minor hole.

Yes, but I think that would require introducing compat_sys_read() and
touching all architectures.

One other thing I just thought about -- the things you were mentioning
wrt. u64 alignment, is that relevant for sendmsg() as well? This doesn't
cover that at all, so far, obviously. If that would be needed, we'd also
need more work there, we can of course do that as we go along and add
code to deal with just the kernel->userspace messages where necessary.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]