All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
@ 2010-04-05 22:27 Florian Westphal
  2010-04-05 22:27 ` [PATCH 1/4] netlink: append NLMSG_DONE to compatskb, too Florian Westphal
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-05 22:27 UTC (permalink / raw)
  To: netdev; +Cc: johannes

3rd version of xfrm x86-compat patch set.

No changes since v2, except patch 5/5, which I dropped from the set.

This has the consequence that compat support is restricted to
applications that use sendmsg() to talk to the kernel (e.g. iproute2
would work); because write() will not go through the socket compat
layer (and thus, the MSG_CMSG_COMPAT won't be set on messages sent
by means of write() ).

I sent a patch that solved this by adding a sys_compat_write syscall
and a ->compat_aio_write() to struct file_operations to the
vfs mailing list, but that patch was ignored by the vfs people,
and the x86 folks did not exactly like the idea either.

So this leaves three alternatives:
1 - drop the whole idea and keep the current status.
2 - Add new structure definitions (with new numbering) that would work
    everywhere, keep the old ones for backwards compatibility (This
    was suggested by Arnd Bergmann).
3 - apply this patch set and tell userspace to move the sendmsg() when
    they want to work with xfrm on x86_64 with 32 bit userland.

Other than that, I am out of ideas.

Patch set description:

At the moment it is not possible to use the xfrm netlink interface on
x86_64 with a 32bit userland.

The problem exists because a few structures, e.g. struct xfrm_usersa_info,
have different sizes in user/kernelspace (3 byte padding on x86, 7
byte on x86_64) due to different alignment requirements of "u64".

First two patches add necessary CONFIG_COMPAT_NETLINK_MESSAGES
infrastructure to netlink in/output path.

Patch 3 is a refactoring patch to split functionality (especially
nlmsg allocation and adding data to the nlmsg) in order to
re-use code and ease review.

Patch 4 adds CONFIG_COMPAT_FOR_U64_ALIGNMENT support to xfrm.

Florian Westphal (4):
      netlink: append NLMSG_DONE to compatskb, too
      netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms
      xfrm: split nlmsg allocation and data copying
      xfrm: CONFIG_COMPAT support for x86 architecture

 include/linux/netlink.h  |    1 +
 net/netlink/af_netlink.c |   11 +
 net/xfrm/Kconfig         |    1 +
 net/xfrm/xfrm_user.c     |  508 ++++++++++++++++++++++++++++++++++++++++------
 4 files changed, 460 insertions(+), 61 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] netlink: append NLMSG_DONE to compatskb, too
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
@ 2010-04-05 22:27 ` Florian Westphal
  2010-04-05 22:27 ` [PATCH 2/4] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms Florian Westphal
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-05 22:27 UTC (permalink / raw)
  To: netdev; +Cc: johannes, Florian Westphal, Florian Westphal

From: Florian Westphal <fwestphal@astaro.com>

modules using netlink may supply a 2nd skb, (via frag_list)
that contains an alternative data set meant for applications
using 32bit compatibility mode.

In such a case, netlink_recvmsg will use this 2nd skb instead of the
original one.

Without this patch, such compat applications will retrieve
all netlink dump data, but will then get an unexpected EOF.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netlink/af_netlink.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 274d977..beaada0 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1709,6 +1709,14 @@ static int netlink_dump(struct sock *sk)
 
 	memcpy(nlmsg_data(nlh), &len, sizeof(len));
 
+#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
+	if (unlikely(skb_shinfo(skb)->frag_list)) {
+		nlh = nlmsg_put_answer(skb_shinfo(skb)->frag_list, cb,
+					NLMSG_DONE, sizeof(len), NLM_F_MULTI);
+		if (nlh)
+			memcpy(nlmsg_data(nlh), &len, sizeof(len));
+	}
+#endif
 	if (sk_filter(sk, skb))
 		kfree_skb(skb);
 	else {
-- 
1.6.4.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
  2010-04-05 22:27 ` [PATCH 1/4] netlink: append NLMSG_DONE to compatskb, too Florian Westphal
@ 2010-04-05 22:27 ` Florian Westphal
  2010-04-05 22:27 ` [PATCH 3/4] xfrm: split nlmsg allocation and data copying Florian Westphal
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-05 22:27 UTC (permalink / raw)
  To: netdev; +Cc: johannes, Florian Westphal

This allows the netlink processing context to determine if the data
needs any 32 bit fixups.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/netlink.h  |    1 +
 net/netlink/af_netlink.c |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index 6eaca5e..031e528 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -164,6 +164,7 @@ struct netlink_skb_parms {
 	__u32			loginuid;	/* Login (audit) uid */
 	__u32			sessionid;	/* Session id (audit) */
 	__u32			sid;		/* SELinux security id */
+	bool			msg_compat;	/* Message needs 32bit fixups */
 };
 
 #define NETLINK_CB(skb)		(*(struct netlink_skb_parms*)&((skb)->cb))
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index beaada0..47c8314 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1339,6 +1339,9 @@ static int netlink_sendmsg(struct kiocb *kiocb, struct socket *sock,
 	NETLINK_CB(skb).dst_group = dst_group;
 	NETLINK_CB(skb).loginuid = audit_get_loginuid(current);
 	NETLINK_CB(skb).sessionid = audit_get_sessionid(current);
+#ifdef CONFIG_COMPAT_NETLINK_MESSAGES
+	NETLINK_CB(skb).msg_compat = !!(msg->msg_flags & MSG_CMSG_COMPAT);
+#endif
 	security_task_getsecid(current, &(NETLINK_CB(skb).sid));
 	memcpy(NETLINK_CREDS(skb), &siocb->scm->creds, sizeof(struct ucred));
 
-- 
1.6.4.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] xfrm: split nlmsg allocation and data copying
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
  2010-04-05 22:27 ` [PATCH 1/4] netlink: append NLMSG_DONE to compatskb, too Florian Westphal
  2010-04-05 22:27 ` [PATCH 2/4] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms Florian Westphal
@ 2010-04-05 22:27 ` Florian Westphal
  2010-04-05 22:27 ` [PATCH 4/4] xfrm: CONFIG_COMPAT support for x86 architecture Florian Westphal
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-05 22:27 UTC (permalink / raw)
  To: netdev; +Cc: johannes, Florian Westphal

To support 32bit userland with different u64 alignment requirements
than a 64bit kernel (COMPAT_FOR_U64_ALIGNMENT), it is
necessary to prepare messages containing affected structures
twice: once in the format expected by 64bit listeners, one
in the format expected by 32bit applications.

In order to minimize copy & pasting and re-use existing
code where possible, split nlmsg allocation and data copying.

Also, replace foo(..., sizeof(*structure)) with

len = sizeof(*structure);
foo(..., len);

so len can be made conditional if we are preparing a compat message.
This will be done in a followup-patch.

With suggestions from Johannes Berg.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/xfrm/xfrm_user.c |  163 ++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 120 insertions(+), 43 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a267fbd..3aba167 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -700,17 +700,17 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
-static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
+static int copy_one_state(struct sk_buff *skb, struct xfrm_state *x,
+			  struct xfrm_dump_info *sp)
 {
-	struct xfrm_dump_info *sp = ptr;
 	struct sk_buff *in_skb = sp->in_skb;
-	struct sk_buff *skb = sp->out_skb;
 	struct xfrm_usersa_info *p;
 	struct nlmsghdr *nlh;
+	size_t len = sizeof(*p);
 	int err;
 
 	nlh = nlmsg_put(skb, NETLINK_CB(in_skb).pid, sp->nlmsg_seq,
-			XFRM_MSG_NEWSA, sizeof(*p), sp->nlmsg_flags);
+			XFRM_MSG_NEWSA, len, sp->nlmsg_flags);
 	if (nlh == NULL)
 		return -EMSGSIZE;
 
@@ -728,6 +728,14 @@ nla_put_failure:
 	return err;
 }
 
+static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
+{
+	struct xfrm_dump_info *sp = ptr;
+	struct sk_buff *skb = sp->out_skb;
+	int ret = copy_one_state(skb, x, sp);
+	return ret;
+}
+
 static int xfrm_dump_sa_done(struct netlink_callback *cb)
 {
 	struct xfrm_state_walk *walk = (struct xfrm_state_walk *) &cb->args[1];
@@ -1359,16 +1367,16 @@ static inline int copy_to_user_policy_type(u8 type, struct sk_buff *skb)
 }
 #endif
 
-static int dump_one_policy(struct xfrm_policy *xp, int dir, int count, void *ptr)
+static int copy_one_policy(struct sk_buff *skb, struct xfrm_policy *xp,
+			   int dir, struct xfrm_dump_info *sp)
 {
-	struct xfrm_dump_info *sp = ptr;
 	struct xfrm_userpolicy_info *p;
 	struct sk_buff *in_skb = sp->in_skb;
-	struct sk_buff *skb = sp->out_skb;
 	struct nlmsghdr *nlh;
+	size_t len = sizeof(*p);
 
 	nlh = nlmsg_put(skb, NETLINK_CB(in_skb).pid, sp->nlmsg_seq,
-			XFRM_MSG_NEWPOLICY, sizeof(*p), sp->nlmsg_flags);
+			XFRM_MSG_NEWPOLICY, len, sp->nlmsg_flags);
 	if (nlh == NULL)
 		return -EMSGSIZE;
 
@@ -1392,6 +1400,15 @@ nlmsg_failure:
 	return -EMSGSIZE;
 }
 
+static int dump_one_policy(struct xfrm_policy *xp, int dir,
+			   int count, void *ptr)
+{
+	struct xfrm_dump_info *sp = ptr;
+	struct sk_buff *skb = sp->out_skb;
+	int ret = copy_one_policy(skb, xp, dir, sp);
+	return ret;
+}
+
 static int xfrm_dump_policy_done(struct netlink_callback *cb)
 {
 	struct xfrm_policy_walk *walk = (struct xfrm_policy_walk *) &cb->args[1];
@@ -1733,7 +1750,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct xfrm_user_polexpire *up = nlmsg_data(nlh);
 	struct xfrm_userpolicy_info *p = &up->pol;
 	u8 type = XFRM_POLICY_TYPE_MAIN;
-	int err = -ENOENT;
+	int hard, err = -ENOENT;
 	struct xfrm_mark m;
 	u32 mark = xfrm_mark_get(attrs, &m);
 
@@ -1774,7 +1791,8 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 
 	err = 0;
-	if (up->hard) {
+	hard = up->hard;
+	if (hard) {
 		uid_t loginuid = NETLINK_CB(skb).loginuid;
 		uid_t sessionid = NETLINK_CB(skb).sessionid;
 		u32 sid = NETLINK_CB(skb).sid;
@@ -1785,7 +1803,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		// reset the timers here?
 		printk("Dont know what to do with soft policy expire\n");
 	}
-	km_policy_expired(xp, p->dir, up->hard, current->pid);
+	km_policy_expired(xp, p->dir, hard, current->pid);
 
 out:
 	xfrm_pol_put(xp);
@@ -1797,7 +1815,7 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 {
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_state *x;
-	int err;
+	int hard, err;
 	struct xfrm_user_expire *ue = nlmsg_data(nlh);
 	struct xfrm_usersa_info *p = &ue->state;
 	struct xfrm_mark m;
@@ -1813,9 +1831,10 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -EINVAL;
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
-	km_state_expired(x, ue->hard, current->pid);
+	hard = ue->hard;
+	km_state_expired(x, hard, current->pid);
 
-	if (ue->hard) {
+	if (hard) {
 		uid_t loginuid = NETLINK_CB(skb).loginuid;
 		uid_t sessionid = NETLINK_CB(skb).sessionid;
 		u32 sid = NETLINK_CB(skb).sid;
@@ -2313,27 +2332,36 @@ static inline size_t xfrm_sa_len(struct xfrm_state *x)
 	return l;
 }
 
-static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
+static int xfrm_notify_sa_len(struct xfrm_state *x, const struct km_event *c)
 {
-	struct net *net = xs_net(x);
-	struct xfrm_usersa_info *p;
-	struct xfrm_usersa_id *id;
-	struct nlmsghdr *nlh;
-	struct sk_buff *skb;
 	int len = xfrm_sa_len(x);
-	int headlen;
+	int headlen = sizeof(struct xfrm_usersa_info);
 
-	headlen = sizeof(*p);
 	if (c->event == XFRM_MSG_DELSA) {
 		len += nla_total_size(headlen);
-		headlen = sizeof(*id);
+		headlen = sizeof(struct xfrm_usersa_id);
 		len += nla_total_size(sizeof(struct xfrm_mark));
 	}
 	len += NLMSG_ALIGN(headlen);
 
-	skb = nlmsg_new(len, GFP_ATOMIC);
-	if (skb == NULL)
-		return -ENOMEM;
+	return len;
+}
+
+static int xfrm_notify_sa_headlen(const struct km_event *c)
+{
+	if (c->event == XFRM_MSG_DELSA)
+		return sizeof(struct xfrm_usersa_id);
+	return sizeof(struct xfrm_usersa_info);
+}
+
+static int copy_to_user_xfrm_notify_sa(struct sk_buff *skb,
+				       struct xfrm_state *x, struct km_event *c)
+{
+	struct xfrm_usersa_info *p;
+	struct xfrm_usersa_id *id;
+	struct nlmsghdr *nlh;
+	int sizeof_usersa_info = sizeof(*p);
+	int headlen = xfrm_notify_sa_headlen(c);
 
 	nlh = nlmsg_put(skb, c->pid, c->seq, c->event, headlen, 0);
 	if (nlh == NULL)
@@ -2349,7 +2377,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
 		id->family = x->props.family;
 		id->proto = x->id.proto;
 
-		attr = nla_reserve(skb, XFRMA_SA, sizeof(*p));
+		attr = nla_reserve(skb, XFRMA_SA, sizeof_usersa_info);
 		if (attr == NULL)
 			goto nla_put_failure;
 
@@ -2360,6 +2388,25 @@ static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
 		goto nla_put_failure;
 
 	nlmsg_end(skb, nlh);
+	return 0;
+nla_put_failure:
+	/* Somebody screwed up with xfrm_sa_len! */
+	WARN_ON(1);
+	return -1;
+}
+
+static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
+{
+	struct sk_buff *skb;
+	struct net *net = xs_net(x);
+	int len = xfrm_notify_sa_len(x, c);
+
+	skb = nlmsg_new(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+
+	if (copy_to_user_xfrm_notify_sa(skb, x, c))
+		goto nla_put_failure;
 
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_SA, GFP_ATOMIC);
 
@@ -2408,10 +2455,11 @@ static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 			 int dir)
 {
 	struct xfrm_user_acquire *ua;
+	size_t len = sizeof(*ua);
 	struct nlmsghdr *nlh;
 	__u32 seq = xfrm_get_acqseq();
 
-	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_ACQUIRE, sizeof(*ua), 0);
+	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_ACQUIRE, len, 0);
 	if (nlh == NULL)
 		return -EMSGSIZE;
 
@@ -2531,10 +2579,11 @@ static int build_polexpire(struct sk_buff *skb, struct xfrm_policy *xp,
 			   int dir, struct km_event *c)
 {
 	struct xfrm_user_polexpire *upe;
+	size_t len = sizeof(*upe);
 	struct nlmsghdr *nlh;
 	int hard = c->data.hard;
 
-	nlh = nlmsg_put(skb, c->pid, 0, XFRM_MSG_POLEXPIRE, sizeof(*upe), 0);
+	nlh = nlmsg_put(skb, c->pid, 0, XFRM_MSG_POLEXPIRE, len, 0);
 	if (nlh == NULL)
 		return -EMSGSIZE;
 
@@ -2573,28 +2622,37 @@ static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, struct km_eve
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_EXPIRE, GFP_ATOMIC);
 }
 
-static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *c)
+static int xfrm_notify_policy_len(struct xfrm_policy *xp, struct km_event *c)
 {
-	struct net *net = xp_net(xp);
-	struct xfrm_userpolicy_info *p;
-	struct xfrm_userpolicy_id *id;
-	struct nlmsghdr *nlh;
-	struct sk_buff *skb;
 	int len = nla_total_size(sizeof(struct xfrm_user_tmpl) * xp->xfrm_nr);
-	int headlen;
+	int headlen = sizeof(struct xfrm_userpolicy_info);
 
-	headlen = sizeof(*p);
 	if (c->event == XFRM_MSG_DELPOLICY) {
 		len += nla_total_size(headlen);
-		headlen = sizeof(*id);
+		headlen = sizeof(struct xfrm_userpolicy_id);
 	}
 	len += userpolicy_type_attrsize();
 	len += nla_total_size(sizeof(struct xfrm_mark));
 	len += NLMSG_ALIGN(headlen);
+	return len;
+}
 
-	skb = nlmsg_new(len, GFP_ATOMIC);
-	if (skb == NULL)
-		return -ENOMEM;
+static int xfrm_notify_policy_headlen(const struct km_event *c)
+{
+	if (c->event == XFRM_MSG_DELPOLICY)
+		return sizeof(struct xfrm_userpolicy_id);
+	return sizeof(struct xfrm_userpolicy_info);
+}
+
+static int copy_to_user_xfrm_notify_policy(struct sk_buff *skb, int dir,
+					   struct xfrm_policy *xp,
+					   struct km_event *c)
+{
+	struct xfrm_userpolicy_info *p;
+	struct xfrm_userpolicy_id *id;
+	struct nlmsghdr *nlh;
+	int sizeof_userpol_info = sizeof(*p);
+	int headlen = xfrm_notify_policy_headlen(c);
 
 	nlh = nlmsg_put(skb, c->pid, c->seq, c->event, headlen, 0);
 	if (nlh == NULL)
@@ -2612,7 +2670,7 @@ static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *
 		else
 			memcpy(&id->sel, &xp->selector, sizeof(id->sel));
 
-		attr = nla_reserve(skb, XFRMA_POLICY, sizeof(*p));
+		attr = nla_reserve(skb, XFRMA_POLICY, sizeof_userpol_info);
 		if (attr == NULL)
 			goto nlmsg_failure;
 
@@ -2630,10 +2688,29 @@ static int xfrm_notify_policy(struct xfrm_policy *xp, int dir, struct km_event *
 
 	nlmsg_end(skb, nlh);
 
-	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_POLICY, GFP_ATOMIC);
+	return 0;
 
 nla_put_failure:
 nlmsg_failure:
+	return -1;
+}
+
+static int xfrm_notify_policy(struct xfrm_policy *xp, int dir,
+			      struct km_event *c)
+{
+	struct net *net = xp_net(xp);
+	struct sk_buff *skb;
+	int len = xfrm_notify_policy_len(xp, c);
+
+	skb = nlmsg_new(len, GFP_ATOMIC);
+	if (skb == NULL)
+		return -ENOMEM;
+	if (copy_to_user_xfrm_notify_policy(skb, dir, xp, c))
+		goto nlmsg_failure;
+
+	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_POLICY, GFP_ATOMIC);
+
+nlmsg_failure:
 	kfree_skb(skb);
 	return -1;
 }
-- 
1.6.4.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] xfrm: CONFIG_COMPAT support for x86 architecture
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
                   ` (2 preceding siblings ...)
  2010-04-05 22:27 ` [PATCH 3/4] xfrm: split nlmsg allocation and data copying Florian Westphal
@ 2010-04-05 22:27 ` Florian Westphal
  2010-04-07 10:08 ` [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support David Miller
  2010-05-13  6:41 ` David Miller
  5 siblings, 0 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-05 22:27 UTC (permalink / raw)
  To: netdev; +Cc: johannes, Florian Westphal

on x86_64, struct xfrm_userpolicy_info/usersa_info have four
additional bytes of padding at the end compared to x86.

Thus, when calling nlmsg_parse(.., sizeof(struct xfrm_userpolicy_info),
trailing attributes are not parsed correctly by the kernel when
the message was sent from an x86 32bit task.

Furthermore, those structures are contained inside
a few other structures, e.g. struct xfrm_user_acquire.

When dealing with incoming data from userland,
those structures need special treatment in the "userland is 32bit"
case.

When sending data to userland, it is sent in both 32bit and native
format.

Errors when building the compat message are not visisble to user
space; data will then be sent without the compat payload.

refer to 1dacc76d0014a034b8aca14237c127d7c19d7726
(net/compat/wext: send different messages to compat tasks) for
more information on netlink compat handling.

With suggestions from Johannes Berg.

Cc: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Florian Westphal <fwestphal@astaro.com>
---
 net/xfrm/Kconfig     |    1 +
 net/xfrm/xfrm_user.c |  369 ++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 340 insertions(+), 30 deletions(-)

diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index 6d08167..0b30357 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -9,6 +9,7 @@ config XFRM
 config XFRM_USER
 	tristate "Transformation user configuration interface"
 	depends on INET && XFRM
+	select WANT_COMPAT_NETLINK_MESSAGES if COMPAT_FOR_U64_ALIGNMENT
 	---help---
 	  Support for Transformation(XFRM) user configuration interface
 	  like IPsec used by native Linux tools.
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 3aba167..6fa9930 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -10,6 +10,7 @@
  *
  */
 
+#include <linux/compat.h>
 #include <linux/crypto.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
@@ -31,6 +32,149 @@
 #include <linux/in6.h>
 #endif
 
+#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
+struct compat_xfrm_lifetime_cfg {
+	compat_u64 soft_byte_limit, hard_byte_limit;
+	compat_u64 soft_packet_limit, hard_packet_limit;
+	compat_u64 soft_add_expires_seconds, hard_add_expires_seconds;
+	compat_u64 soft_use_expires_seconds, hard_use_expires_seconds;
+};
+
+struct compat_xfrm_lifetime_cur {
+	compat_u64 bytes, packets, add_time, use_time;
+};
+
+struct compat_xfrm_userpolicy_info {
+	struct xfrm_selector sel;
+	struct compat_xfrm_lifetime_cfg lft;
+	struct compat_xfrm_lifetime_cur curlft;
+	u32 priority, index;
+	u8 dir, action, flags, share;
+	/* 4 bytes additional padding on 64bit */
+};
+
+struct compat_xfrm_usersa_info {
+	struct xfrm_selector sel;
+	struct xfrm_id id;
+	xfrm_address_t saddr;
+	struct compat_xfrm_lifetime_cfg lft;
+	struct compat_xfrm_lifetime_cur curlft;
+	struct xfrm_stats stats;
+	u32 seq, reqid;
+	u16 family;
+	u8 mode, replay_window, flags;
+	/* 4 bytes additional padding on 64bit */
+};
+
+struct compat_xfrm_user_acquire {
+	struct xfrm_id id;
+	xfrm_address_t saddr;
+	struct xfrm_selector sel;
+	struct compat_xfrm_userpolicy_info policy;
+	/* 4 bytes additional padding on 64bit */
+	u32 aalgos, ealgos, calgos, seq;
+};
+
+struct compat_xfrm_userspi_info {
+	struct compat_xfrm_usersa_info info;
+	/* 4 bytes additional padding on 64bit */
+	u32 min, max;
+};
+
+struct compat_xfrm_user_expire {
+	struct compat_xfrm_usersa_info state;
+	/* 4 bytes additional padding on 64bit */
+	u8 hard;
+};
+
+struct compat_xfrm_user_polexpire {
+	struct compat_xfrm_userpolicy_info pol;
+	/* 4 bytes additional padding on 64bit */
+	u8 hard;
+};
+
+static bool xfrm_msg_compat(const struct sk_buff *skb)
+{
+	return unlikely(NETLINK_CB(skb).msg_compat);
+}
+
+static struct sk_buff *xfrm_add_compatskb(struct sk_buff *skb,
+					  unsigned int len, gfp_t gfp)
+{
+	struct sk_buff *compatskb = nlmsg_new(len, gfp);
+
+	WARN_ON(skb_shinfo(skb)->frag_list);
+
+	skb_shinfo(skb)->frag_list = compatskb;
+	return compatskb;
+}
+#else
+static inline bool xfrm_msg_compat(const struct sk_buff *skb)
+{
+	return false;
+}
+
+static inline struct sk_buff *xfrm_add_compatskb(struct sk_buff *skb,
+						 unsigned int len, gfp_t gfp)
+{
+	return NULL;
+}
+
+/*
+ * avoids #ifdefs all over the place. Use of these must be conditional via
+ * xfrm_msg_compat/xfrm_add_compatskb so compiler can remove branches.
+ */
+#define compat_xfrm_user_expire     xfrm_user_expire
+#define compat_xfrm_user_acquire    xfrm_user_acquire
+#define compat_xfrm_user_polexpire  xfrm_user_polexpire
+#define compat_xfrm_userpolicy_info xfrm_userpolicy_info
+#define compat_xfrm_usersa_info     xfrm_usersa_info
+#define compat_xfrm_userspi_info    xfrm_userspi_info
+
+#endif /* CONFIG_COMPAT_FOR_U64_ALIGNMENT */
+
+/*
+ * userspace size of some structures is smaller due to different u64
+ * u64 alignment on x86 platform.
+ *
+ * Some of the structures need to use the compat_* structure definition
+ * when accessing certain members, see compat_ structures above.
+ */
+#define XMSGDELTA(type) (sizeof(struct type) - sizeof(struct compat_##type))
+static const u8 xfrm_msg_min_compat_pad[XFRM_NR_MSGTYPES] = {
+	[XFRM_MSG_NEWSA     - XFRM_MSG_BASE] = XMSGDELTA(xfrm_usersa_info),
+	[XFRM_MSG_NEWPOLICY - XFRM_MSG_BASE] = XMSGDELTA(xfrm_userpolicy_info),
+	[XFRM_MSG_ALLOCSPI  - XFRM_MSG_BASE] = XMSGDELTA(xfrm_userspi_info),
+	[XFRM_MSG_ACQUIRE   - XFRM_MSG_BASE] = XMSGDELTA(xfrm_user_acquire),
+	[XFRM_MSG_EXPIRE    - XFRM_MSG_BASE] = XMSGDELTA(xfrm_user_expire),
+	[XFRM_MSG_UPDPOLICY - XFRM_MSG_BASE] = XMSGDELTA(xfrm_userpolicy_info),
+	[XFRM_MSG_UPDSA     - XFRM_MSG_BASE] = XMSGDELTA(xfrm_usersa_info),
+	[XFRM_MSG_POLEXPIRE - XFRM_MSG_BASE] = XMSGDELTA(xfrm_user_polexpire),
+};
+#undef XMSGDELTA
+
+static int get_user_expire_hard(const struct sk_buff *skb,
+				const struct xfrm_user_expire *ue)
+{
+	if (xfrm_msg_compat(skb)) {
+		const struct compat_xfrm_user_expire *cmpt;
+		cmpt = (const struct compat_xfrm_user_expire *) ue;
+		return cmpt->hard;
+	}
+	return ue->hard;
+}
+
+static int get_user_polexpire_hard(const struct sk_buff *skb,
+				   const struct xfrm_user_polexpire *ue)
+{
+	if (xfrm_msg_compat(skb)) {
+		const struct compat_xfrm_user_polexpire *cmpt;
+		cmpt = (const struct compat_xfrm_user_polexpire *) ue;
+		return cmpt->hard;
+	}
+	return ue->hard;
+}
+
 static inline int aead_len(struct xfrm_algo_aead *alg)
 {
 	return sizeof(*alg) + ((alg->alg_key_len + 7) / 8);
@@ -701,7 +845,7 @@ nla_put_failure:
 }
 
 static int copy_one_state(struct sk_buff *skb, struct xfrm_state *x,
-			  struct xfrm_dump_info *sp)
+			  struct xfrm_dump_info *sp, bool compat)
 {
 	struct sk_buff *in_skb = sp->in_skb;
 	struct xfrm_usersa_info *p;
@@ -709,6 +853,9 @@ static int copy_one_state(struct sk_buff *skb, struct xfrm_state *x,
 	size_t len = sizeof(*p);
 	int err;
 
+	if (compat)
+		len = sizeof(struct compat_xfrm_usersa_info);
+
 	nlh = nlmsg_put(skb, NETLINK_CB(in_skb).pid, sp->nlmsg_seq,
 			XFRM_MSG_NEWSA, len, sp->nlmsg_flags);
 	if (nlh == NULL)
@@ -732,7 +879,14 @@ static int dump_one_state(struct xfrm_state *x, int count, void *ptr)
 {
 	struct xfrm_dump_info *sp = ptr;
 	struct sk_buff *skb = sp->out_skb;
-	int ret = copy_one_state(skb, x, sp);
+	int ret = copy_one_state(skb, x, sp, false);
+#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
+	if (ret == 0) {
+		skb = skb_shinfo(skb)->frag_list;
+		if (skb)
+			copy_one_state(skb, x, sp, true);
+	}
+#endif
 	return ret;
 }
 
@@ -762,6 +916,8 @@ static int xfrm_dump_sa(struct sk_buff *skb, struct netlink_callback *cb)
 		xfrm_state_walk_init(walk, 0);
 	}
 
+	xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+
 	(void) xfrm_state_walk(net, walk, dump_one_state, &info);
 
 	return skb->len;
@@ -782,6 +938,8 @@ static struct sk_buff *xfrm_state_netlink(struct sk_buff *in_skb,
 	info.nlmsg_seq = seq;
 	info.nlmsg_flags = 0;
 
+	xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+
 	if (dump_one_state(x, 0, &info)) {
 		kfree_skb(skb);
 		return NULL;
@@ -953,6 +1111,29 @@ static int verify_userspi_info(struct xfrm_userspi_info *p)
 	return 0;
 }
 
+static int compat_verify_userspi_info(struct xfrm_userspi_info *p)
+{
+	struct compat_xfrm_userspi_info *compat;
+
+	compat = (struct compat_xfrm_userspi_info  *) p;
+
+	switch (p->info.id.proto) {
+	case IPPROTO_AH:
+	case IPPROTO_ESP:
+		break;
+	case IPPROTO_COMP:
+		if (compat->max >= 0x10000)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (compat->min > compat->max)
+		return -EINVAL;
+	return 0;
+}
+
 static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct nlattr **attrs)
 {
@@ -967,7 +1148,10 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct xfrm_mark m;
 
 	p = nlmsg_data(nlh);
-	err = verify_userspi_info(p);
+	if (xfrm_msg_compat(skb))
+		err = compat_verify_userspi_info(p);
+	else
+		err = verify_userspi_info(p);
 	if (err)
 		goto out_noput;
 
@@ -993,8 +1177,14 @@ static int xfrm_alloc_userspi(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -ENOENT;
 	if (x == NULL)
 		goto out_noput;
+	if (xfrm_msg_compat(skb)) {
+		struct compat_xfrm_userspi_info *compat;
+		compat = (struct compat_xfrm_userspi_info *) p;
+		err = xfrm_alloc_spi(x, compat->min, compat->max);
+	} else {
+		err = xfrm_alloc_spi(x, p->min, p->max);
+	}
 
-	err = xfrm_alloc_spi(x, p->min, p->max);
 	if (err)
 		goto out;
 
@@ -1368,13 +1558,16 @@ static inline int copy_to_user_policy_type(u8 type, struct sk_buff *skb)
 #endif
 
 static int copy_one_policy(struct sk_buff *skb, struct xfrm_policy *xp,
-			   int dir, struct xfrm_dump_info *sp)
+			   int dir, struct xfrm_dump_info *sp, bool compat)
 {
 	struct xfrm_userpolicy_info *p;
 	struct sk_buff *in_skb = sp->in_skb;
 	struct nlmsghdr *nlh;
 	size_t len = sizeof(*p);
 
+	if (compat)
+		len = sizeof(struct compat_xfrm_userpolicy_info);
+
 	nlh = nlmsg_put(skb, NETLINK_CB(in_skb).pid, sp->nlmsg_seq,
 			XFRM_MSG_NEWPOLICY, len, sp->nlmsg_flags);
 	if (nlh == NULL)
@@ -1405,7 +1598,14 @@ static int dump_one_policy(struct xfrm_policy *xp, int dir,
 {
 	struct xfrm_dump_info *sp = ptr;
 	struct sk_buff *skb = sp->out_skb;
-	int ret = copy_one_policy(skb, xp, dir, sp);
+	int ret = copy_one_policy(skb, xp, dir, sp, false);
+#ifdef CONFIG_COMPAT_FOR_U64_ALIGNMENT
+	if (ret == 0) {
+		skb = skb_shinfo(skb)->frag_list;
+		if (skb)
+			copy_one_policy(skb, xp, dir, sp, true);
+	}
+#endif
 	return ret;
 }
 
@@ -1436,6 +1636,8 @@ static int xfrm_dump_policy(struct sk_buff *skb, struct netlink_callback *cb)
 		xfrm_policy_walk_init(walk, XFRM_POLICY_TYPE_ANY);
 	}
 
+	xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+
 	(void) xfrm_policy_walk(net, walk, dump_one_policy, &info);
 
 	return skb->len;
@@ -1457,6 +1659,8 @@ static struct sk_buff *xfrm_policy_netlink(struct sk_buff *in_skb,
 	info.nlmsg_seq = seq;
 	info.nlmsg_flags = 0;
 
+	xfrm_add_compatskb(skb, NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+
 	if (dump_one_policy(xp, dir, 0, &info) < 0) {
 		kfree_skb(skb);
 		return NULL;
@@ -1791,7 +1995,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 
 	err = 0;
-	hard = up->hard;
+	hard = get_user_polexpire_hard(skb, up);
 	if (hard) {
 		uid_t loginuid = NETLINK_CB(skb).loginuid;
 		uid_t sessionid = NETLINK_CB(skb).sessionid;
@@ -1831,7 +2035,7 @@ static int xfrm_add_sa_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	err = -EINVAL;
 	if (x->km.state != XFRM_STATE_VALID)
 		goto out;
-	hard = ue->hard;
+	hard = get_user_expire_hard(skb, ue);
 	km_state_expired(x, hard, current->pid);
 
 	if (hard) {
@@ -1848,6 +2052,23 @@ out:
 	return err;
 }
 
+static void acquire_extract_algos(const struct sk_buff *skb,
+				   struct xfrm_tmpl *t,
+				   const struct xfrm_user_acquire *ua)
+{
+	if (!xfrm_msg_compat(skb)) {
+		t->aalgos = ua->aalgos;
+		t->ealgos = ua->ealgos;
+		t->calgos = ua->calgos;
+	} else {
+		const struct compat_xfrm_user_acquire *compat_ua;
+		compat_ua = (const struct compat_xfrm_user_acquire *) ua;
+		t->aalgos = compat_ua->aalgos;
+		t->ealgos = compat_ua->ealgos;
+		t->calgos = compat_ua->calgos;
+	}
+}
+
 static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		struct nlattr **attrs)
 {
@@ -1889,9 +2110,7 @@ static int xfrm_add_acquire(struct sk_buff *skb, struct nlmsghdr *nlh,
 		x->props.mode = t->mode;
 		x->props.reqid = t->reqid;
 		x->props.family = ut->family;
-		t->aalgos = ua->aalgos;
-		t->ealgos = ua->ealgos;
-		t->calgos = ua->calgos;
+		acquire_extract_algos(skb, t, ua);
 		err = km_query(x, t, xp);
 
 	}
@@ -2173,6 +2392,13 @@ static struct xfrm_link {
 	[XFRM_MSG_GETSPDINFO  - XFRM_MSG_BASE] = { .doit = xfrm_get_spdinfo   },
 };
 
+static int xfrm_msg_hdrlen(const struct sk_buff *skb, int type)
+{
+	if (xfrm_msg_compat(skb))
+		return xfrm_msg_min[type] - xfrm_msg_min_compat_pad[type];
+	return xfrm_msg_min[type];
+}
+
 static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	struct net *net = sock_net(skb->sk);
@@ -2200,7 +2426,7 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		return netlink_dump_start(net->xfrm.nlsk, skb, nlh, link->dump, link->done);
 	}
 
-	err = nlmsg_parse(nlh, xfrm_msg_min[type], attrs, XFRMA_MAX,
+	err = nlmsg_parse(nlh, xfrm_msg_hdrlen(skb, type), attrs, XFRMA_MAX,
 			  xfrma_policy);
 	if (err < 0)
 		return err;
@@ -2246,10 +2472,27 @@ nla_put_failure:
 	return -EMSGSIZE;
 }
 
+static void compat_build_expire(struct sk_buff *skb, struct xfrm_state *x,
+				struct km_event *c)
+{
+	struct compat_xfrm_user_expire *compat_ue;
+	struct nlmsghdr *nlh;
+
+	nlh = nlmsg_put(skb, c->pid, 0, XFRM_MSG_EXPIRE, sizeof(*compat_ue), 0);
+	if (nlh == NULL)
+		return;
+
+	compat_ue = nlmsg_data(nlh);
+	copy_to_user_state(x, (struct xfrm_usersa_info *) &compat_ue->state);
+	compat_ue->hard = (c->data.hard != 0) ? 1 : 0;
+
+	nlmsg_end(skb, nlh);
+}
+
 static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 {
 	struct net *net = xs_net(x);
-	struct sk_buff *skb;
+	struct sk_buff *skb, *cskb;
 
 	skb = nlmsg_new(xfrm_expire_msgsize(), GFP_ATOMIC);
 	if (skb == NULL)
@@ -2260,6 +2503,9 @@ static int xfrm_exp_state_notify(struct xfrm_state *x, struct km_event *c)
 		return -EMSGSIZE;
 	}
 
+	if ((cskb = xfrm_add_compatskb(skb, xfrm_expire_msgsize(), GFP_ATOMIC)))
+		compat_build_expire(cskb, x, c);
+
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_EXPIRE, GFP_ATOMIC);
 }
 
@@ -2354,8 +2600,16 @@ static int xfrm_notify_sa_headlen(const struct km_event *c)
 	return sizeof(struct xfrm_usersa_info);
 }
 
+static int compat_xfrm_notify_sa_headlen(const struct km_event *c)
+{
+	if (c->event == XFRM_MSG_DELSA)
+		return sizeof(struct xfrm_usersa_id);
+	return sizeof(struct compat_xfrm_usersa_info);
+}
+
 static int copy_to_user_xfrm_notify_sa(struct sk_buff *skb,
-				       struct xfrm_state *x, struct km_event *c)
+				       struct xfrm_state *x,
+				       struct km_event *c, bool compat)
 {
 	struct xfrm_usersa_info *p;
 	struct xfrm_usersa_id *id;
@@ -2363,6 +2617,11 @@ static int copy_to_user_xfrm_notify_sa(struct sk_buff *skb,
 	int sizeof_usersa_info = sizeof(*p);
 	int headlen = xfrm_notify_sa_headlen(c);
 
+	if (compat) {
+		headlen = compat_xfrm_notify_sa_headlen(c);
+		sizeof_usersa_info = sizeof(struct compat_xfrm_usersa_info);
+	}
+
 	nlh = nlmsg_put(skb, c->pid, c->seq, c->event, headlen, 0);
 	if (nlh == NULL)
 		goto nla_put_failure;
@@ -2397,7 +2656,7 @@ nla_put_failure:
 
 static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
 {
-	struct sk_buff *skb;
+	struct sk_buff *skb, *cskb;
 	struct net *net = xs_net(x);
 	int len = xfrm_notify_sa_len(x, c);
 
@@ -2405,9 +2664,12 @@ static int xfrm_notify_sa(struct xfrm_state *x, struct km_event *c)
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (copy_to_user_xfrm_notify_sa(skb, x, c))
+	if (copy_to_user_xfrm_notify_sa(skb, x, c, false))
 		goto nla_put_failure;
 
+	if ((cskb = xfrm_add_compatskb(skb, len, GFP_ATOMIC)))
+		copy_to_user_xfrm_notify_sa(cskb, x, c, true);
+
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_SA, GFP_ATOMIC);
 
 nla_put_failure:
@@ -2452,12 +2714,16 @@ static inline size_t xfrm_acquire_msgsize(struct xfrm_state *x,
 
 static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 			 struct xfrm_tmpl *xt, struct xfrm_policy *xp,
-			 int dir)
+			 int dir, bool compat)
 {
 	struct xfrm_user_acquire *ua;
 	size_t len = sizeof(*ua);
 	struct nlmsghdr *nlh;
 	__u32 seq = xfrm_get_acqseq();
+	struct compat_xfrm_user_acquire *compat_ua;
+
+	if (compat)
+		len = sizeof(*compat_ua);
 
 	nlh = nlmsg_put(skb, 0, 0, XFRM_MSG_ACQUIRE, len, 0);
 	if (nlh == NULL)
@@ -2468,10 +2734,19 @@ static int build_acquire(struct sk_buff *skb, struct xfrm_state *x,
 	memcpy(&ua->saddr, &x->props.saddr, sizeof(ua->saddr));
 	memcpy(&ua->sel, &x->sel, sizeof(ua->sel));
 	copy_to_user_policy(xp, &ua->policy, dir);
-	ua->aalgos = xt->aalgos;
-	ua->ealgos = xt->ealgos;
-	ua->calgos = xt->calgos;
-	ua->seq = x->km.seq = seq;
+
+	if (compat) {
+		compat_ua = nlmsg_data(nlh);
+		compat_ua->aalgos = xt->aalgos;
+		compat_ua->ealgos = xt->ealgos;
+		compat_ua->calgos = xt->calgos;
+		compat_ua->seq = x->km.seq = seq;
+	} else {
+		ua->aalgos = xt->aalgos;
+		ua->ealgos = xt->ealgos;
+		ua->calgos = xt->calgos;
+		ua->seq = x->km.seq = seq;
+	}
 
 	if (copy_to_user_tmpl(xp, skb) < 0)
 		goto nlmsg_failure;
@@ -2494,15 +2769,19 @@ static int xfrm_send_acquire(struct xfrm_state *x, struct xfrm_tmpl *xt,
 			     struct xfrm_policy *xp, int dir)
 {
 	struct net *net = xs_net(x);
-	struct sk_buff *skb;
+	struct sk_buff *skb, *cskb;
 
 	skb = nlmsg_new(xfrm_acquire_msgsize(x, xp), GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_acquire(skb, x, xt, xp, dir) < 0)
+	if (build_acquire(skb, x, xt, xp, dir, false) < 0)
 		BUG();
 
+	cskb = xfrm_add_compatskb(skb, xfrm_acquire_msgsize(x, xp), GFP_ATOMIC);
+	if (cskb)
+		build_acquire(cskb, x, xt, xp, dir, true);
+
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_ACQUIRE, GFP_ATOMIC);
 }
 
@@ -2576,12 +2855,16 @@ static inline size_t xfrm_polexpire_msgsize(struct xfrm_policy *xp)
 }
 
 static int build_polexpire(struct sk_buff *skb, struct xfrm_policy *xp,
-			   int dir, struct km_event *c)
+			   int dir, struct km_event *c, bool compat)
 {
 	struct xfrm_user_polexpire *upe;
 	size_t len = sizeof(*upe);
 	struct nlmsghdr *nlh;
 	int hard = c->data.hard;
+	struct compat_xfrm_user_polexpire *upe_cmpt;
+
+	if (compat)
+		len = sizeof(*upe_cmpt);
 
 	nlh = nlmsg_put(skb, c->pid, 0, XFRM_MSG_POLEXPIRE, len, 0);
 	if (nlh == NULL)
@@ -2597,7 +2880,13 @@ static int build_polexpire(struct sk_buff *skb, struct xfrm_policy *xp,
 		goto nlmsg_failure;
 	if (xfrm_mark_put(skb, &xp->mark))
 		goto nla_put_failure;
-	upe->hard = !!hard;
+
+	if (compat) {
+		upe_cmpt = nlmsg_data(nlh);
+		upe_cmpt->hard = !!hard;
+	} else {
+		upe->hard = !!hard;
+	}
 
 	return nlmsg_end(skb, nlh);
 
@@ -2610,15 +2899,19 @@ nlmsg_failure:
 static int xfrm_exp_policy_notify(struct xfrm_policy *xp, int dir, struct km_event *c)
 {
 	struct net *net = xp_net(xp);
-	struct sk_buff *skb;
+	struct sk_buff *skb, *cskb;
 
 	skb = nlmsg_new(xfrm_polexpire_msgsize(xp), GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
 
-	if (build_polexpire(skb, xp, dir, c) < 0)
+	if (build_polexpire(skb, xp, dir, c, false) < 0)
 		BUG();
 
+	cskb = xfrm_add_compatskb(skb, xfrm_polexpire_msgsize(xp), GFP_ATOMIC);
+	if (cskb)
+		build_polexpire(cskb, xp, dir, c, true);
+
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_EXPIRE, GFP_ATOMIC);
 }
 
@@ -2644,9 +2937,16 @@ static int xfrm_notify_policy_headlen(const struct km_event *c)
 	return sizeof(struct xfrm_userpolicy_info);
 }
 
+static int compat_xfrm_notify_policy_headlen(const struct km_event *c)
+{
+	if (c->event == XFRM_MSG_DELPOLICY)
+		return sizeof(struct xfrm_userpolicy_id);
+	return sizeof(struct compat_xfrm_userpolicy_info);
+}
+
 static int copy_to_user_xfrm_notify_policy(struct sk_buff *skb, int dir,
 					   struct xfrm_policy *xp,
-					   struct km_event *c)
+					   struct km_event *c, bool compat)
 {
 	struct xfrm_userpolicy_info *p;
 	struct xfrm_userpolicy_id *id;
@@ -2654,6 +2954,12 @@ static int copy_to_user_xfrm_notify_policy(struct sk_buff *skb, int dir,
 	int sizeof_userpol_info = sizeof(*p);
 	int headlen = xfrm_notify_policy_headlen(c);
 
+	if (compat) {
+		sizeof_userpol_info =
+				sizeof(struct compat_xfrm_userpolicy_info);
+		headlen = compat_xfrm_notify_policy_headlen(c);
+	}
+
 	nlh = nlmsg_put(skb, c->pid, c->seq, c->event, headlen, 0);
 	if (nlh == NULL)
 		goto nlmsg_failure;
@@ -2699,15 +3005,18 @@ static int xfrm_notify_policy(struct xfrm_policy *xp, int dir,
 			      struct km_event *c)
 {
 	struct net *net = xp_net(xp);
-	struct sk_buff *skb;
+	struct sk_buff *skb, *cskb;
 	int len = xfrm_notify_policy_len(xp, c);
 
 	skb = nlmsg_new(len, GFP_ATOMIC);
 	if (skb == NULL)
 		return -ENOMEM;
-	if (copy_to_user_xfrm_notify_policy(skb, dir, xp, c))
+	if (copy_to_user_xfrm_notify_policy(skb, dir, xp, c, false))
 		goto nlmsg_failure;
 
+	if ((cskb = xfrm_add_compatskb(skb, len, GFP_ATOMIC)))
+		copy_to_user_xfrm_notify_policy(cskb, dir, xp, c, true);
+
 	return nlmsg_multicast(net->xfrm.nlsk, skb, 0, XFRMNLGRP_POLICY, GFP_ATOMIC);
 
 nlmsg_failure:
-- 
1.6.4.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
                   ` (3 preceding siblings ...)
  2010-04-05 22:27 ` [PATCH 4/4] xfrm: CONFIG_COMPAT support for x86 architecture Florian Westphal
@ 2010-04-07 10:08 ` David Miller
  2010-04-07 13:35   ` Florian Westphal
  2010-05-13  6:41 ` David Miller
  5 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2010-04-07 10:08 UTC (permalink / raw)
  To: fw; +Cc: netdev, johannes

From: Florian Westphal <fw@strlen.de>
Date: Tue,  6 Apr 2010 00:27:07 +0200

> This has the consequence that compat support is restricted to
> applications that use sendmsg() to talk to the kernel (e.g. iproute2
> would work); because write() will not go through the socket compat
> layer (and thus, the MSG_CMSG_COMPAT won't be set on messages sent
> by means of write() ).
> 
> I sent a patch that solved this by adding a sys_compat_write syscall
> and a ->compat_aio_write() to struct file_operations to the
> vfs mailing list, but that patch was ignored by the vfs people,
> and the x86 folks did not exactly like the idea either.
> 
> So this leaves three alternatives:
> 1 - drop the whole idea and keep the current status.
> 2 - Add new structure definitions (with new numbering) that would work
>     everywhere, keep the old ones for backwards compatibility (This
>     was suggested by Arnd Bergmann).
> 3 - apply this patch set and tell userspace to move the sendmsg() when
>     they want to work with xfrm on x86_64 with 32 bit userland.
> 
> Other than that, I am out of ideas.

So do we know of any xfrm netlink apps that do not use sendmsg()?

I doubt there are any, and if that's true for the most part, then
option #3 seems the best.

We simply can't ignore all of the 32-bit xfrm netlink app binaries out
there and pretend they don't exist.  So #2 doesn't make any sense to
me at all.

And #1, not trying to make it work at all, to me is just as bad
as #2.

If we find a non-trivial number of apps using plain write() then
we might have to consider championing your vfs patch to the
lkml and vfs folks again.  I'll help if this is needed.

Please do some quick research on xfrm netlink sendmsg() vs.
write() usage and based upon your findings we'll act.

Thanks for keeping this work alive.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-07 10:08 ` [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support David Miller
@ 2010-04-07 13:35   ` Florian Westphal
  2010-04-07 13:45     ` Patrick McHardy
  2010-04-08  5:00     ` David Miller
  0 siblings, 2 replies; 14+ messages in thread
From: Florian Westphal @ 2010-04-07 13:35 UTC (permalink / raw)
  To: David Miller; +Cc: fw, netdev, johannes

David Miller <davem@davemloft.net> wrote:
> From: Florian Westphal <fw@strlen.de>
> Date: Tue,  6 Apr 2010 00:27:07 +0200

[..]

> > I sent a patch that solved this by adding a sys_compat_write syscall
> > and a ->compat_aio_write() to struct file_operations to the
> > vfs mailing list, but that patch was ignored by the vfs people,
> > and the x86 folks did not exactly like the idea either.
> >
> > So this leaves three alternatives:
> > 1 - drop the whole idea and keep the current status.
> > 2 - Add new structure definitions (with new numbering) that would work
> >     everywhere, keep the old ones for backwards compatibility (This
> >     was suggested by Arnd Bergmann).
> > 3 - apply this patch set and tell userspace to move the sendmsg() when
> >     they want to work with xfrm on x86_64 with 32 bit userland.
>
> So do we know of any xfrm netlink apps that do not use sendmsg()?
>
> I doubt there are any, and if that's true for the most part, then
> option #3 seems the best.

Open- and strongswan do (in pluto/kernel_netlink.c), strongswan also uses
sendto() in charon (their ikev2 daemon); sendto does not work at the
moment either.

That being said, this is only in a single spot.
The openswan people indicated they'd accept a patch that converts the
write to a sendmsg call.

openikev2 also uses write on a netlink socket (looks rather similar
to the one in open/strongswan).

AFAICS, all the receive functions use recvfrom (which sets CMSG_COMPAT).

I did not find other ike software that uses xfrm.

> If we find a non-trivial number of apps using plain write() then
> we might have to consider championing your vfs patch to the
> lkml and vfs folks again.

There is probably no "nontrivial" number, simply because there are not
that many user apps out there to begin with.

But whats really bothering me is the number of sys_compat_* functions
needed to make all possibilities work;. e.g. to make (unmodified)
strongwan work, sys_compat_write and sys_compat_sendto are needed.

What about applications that use any of the other countless
write functions?  Right now these do not set CMSG_COMPAT either.

And lets not get started with the abdunance of read() like functions...

[ I do not know about any such applications, but still ...]

> I'll help if this is needed.

Thank you.

Personally I hope that we can move the affected userspace tools to
sendmsg() and avoid introducing new compat syscalls, simply because
there is no telling what kind of mess we'd be walking into :-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-07 13:35   ` Florian Westphal
@ 2010-04-07 13:45     ` Patrick McHardy
  2010-04-07 23:48       ` David Miller
  2010-04-08  5:00     ` David Miller
  1 sibling, 1 reply; 14+ messages in thread
From: Patrick McHardy @ 2010-04-07 13:45 UTC (permalink / raw)
  To: Florian Westphal; +Cc: David Miller, netdev, johannes

Florian Westphal wrote:
> David Miller <davem@davemloft.net> wrote:
>> From: Florian Westphal <fw@strlen.de>
>> Date: Tue,  6 Apr 2010 00:27:07 +0200
> 
> [..]
> 
>>> I sent a patch that solved this by adding a sys_compat_write syscall
>>> and a ->compat_aio_write() to struct file_operations to the
>>> vfs mailing list, but that patch was ignored by the vfs people,
>>> and the x86 folks did not exactly like the idea either.
>>>
>>> So this leaves three alternatives:
>>> 1 - drop the whole idea and keep the current status.
>>> 2 - Add new structure definitions (with new numbering) that would work
>>>     everywhere, keep the old ones for backwards compatibility (This
>>>     was suggested by Arnd Bergmann).

Given that there is only a quite small number of users of this
interface, that would in my opinion be the best way.

>>> 3 - apply this patch set and tell userspace to move the sendmsg() when
>>>     they want to work with xfrm on x86_64 with 32 bit userland.
>> So do we know of any xfrm netlink apps that do not use sendmsg()?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-07 13:45     ` Patrick McHardy
@ 2010-04-07 23:48       ` David Miller
  2010-04-08  9:44         ` Patrick McHardy
  0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2010-04-07 23:48 UTC (permalink / raw)
  To: kaber; +Cc: fw, netdev, johannes

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 07 Apr 2010 15:45:51 +0200

> Florian Westphal wrote:
>> David Miller <davem@davemloft.net> wrote:
>>> From: Florian Westphal <fw@strlen.de>
>>> Date: Tue,  6 Apr 2010 00:27:07 +0200
>> 
>> [..]
>> 
>>>> I sent a patch that solved this by adding a sys_compat_write syscall
>>>> and a ->compat_aio_write() to struct file_operations to the
>>>> vfs mailing list, but that patch was ignored by the vfs people,
>>>> and the x86 folks did not exactly like the idea either.
>>>>
>>>> So this leaves three alternatives:
>>>> 1 - drop the whole idea and keep the current status.
>>>> 2 - Add new structure definitions (with new numbering) that would work
>>>>     everywhere, keep the old ones for backwards compatibility (This
>>>>     was suggested by Arnd Bergmann).
> 
> Given that there is only a quite small number of users of this
> interface, that would in my opinion be the best way.

Can you explain that line of reasoning?

It's not that there are only "3 or 4 tools" using these interfaces,
it's the fact that 32-bit binaries of those tools are on millions and
millions of systems out there.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-07 13:35   ` Florian Westphal
  2010-04-07 13:45     ` Patrick McHardy
@ 2010-04-08  5:00     ` David Miller
  1 sibling, 0 replies; 14+ messages in thread
From: David Miller @ 2010-04-08  5:00 UTC (permalink / raw)
  To: fw; +Cc: netdev, johannes

From: Florian Westphal <fw@strlen.de>
Date: Wed, 7 Apr 2010 15:35:28 +0200

> But whats really bothering me is the number of sys_compat_* functions
> needed to make all possibilities work;. e.g. to make (unmodified)
> strongwan work, sys_compat_write and sys_compat_sendto are needed.

Thank the BSD socket API designer(s) for adding N different ways to
essentially say the same thing instead of just having sendmsg/recvmsg
and saying "if you don't want to specify X, just pass in NULL" or
whatever. :-)

Because that's all that sendto() is, it's a sendmsg() without
an arg or two.  But in the end the kernel internally has to
come up with pretend arguments for all of this stuff anyways
since the protocols end up having to accomodate all of
the sendmsg() cases anyways.

And it's not all that bad, we just need the numerous compat entry
points to set the compat flag bit, the rest of the implementation is
going to be %100 shared code.

And once it's there, we can use it for other similar cases, not
just xfrm netlink.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-07 23:48       ` David Miller
@ 2010-04-08  9:44         ` Patrick McHardy
  2010-04-08  9:54           ` David Miller
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick McHardy @ 2010-04-08  9:44 UTC (permalink / raw)
  To: David Miller; +Cc: fw, netdev, johannes

David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Wed, 07 Apr 2010 15:45:51 +0200
> 
>> Florian Westphal wrote:
>>> David Miller <davem@davemloft.net> wrote:
>>>> From: Florian Westphal <fw@strlen.de>
>>>> Date: Tue,  6 Apr 2010 00:27:07 +0200
>>> [..]
>>>
>>>>> I sent a patch that solved this by adding a sys_compat_write syscall
>>>>> and a ->compat_aio_write() to struct file_operations to the
>>>>> vfs mailing list, but that patch was ignored by the vfs people,
>>>>> and the x86 folks did not exactly like the idea either.
>>>>>
>>>>> So this leaves three alternatives:
>>>>> 1 - drop the whole idea and keep the current status.
>>>>> 2 - Add new structure definitions (with new numbering) that would work
>>>>>     everywhere, keep the old ones for backwards compatibility (This
>>>>>     was suggested by Arnd Bergmann).
>> Given that there is only a quite small number of users of this
>> interface, that would in my opinion be the best way.
> 
> Can you explain that line of reasoning?
> 
> It's not that there are only "3 or 4 tools" using these interfaces,
> it's the fact that 32-bit binaries of those tools are on millions and
> millions of systems out there.

Yeah, but they're currently not working if used on 64 bit hosts,
so I don't think its unreasonable to consider just the number of
tools that need to be fixed. Either the kernel or the userspace
programs have to be updated either way.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-08  9:44         ` Patrick McHardy
@ 2010-04-08  9:54           ` David Miller
  2010-04-08 11:24             ` Patrick McHardy
  0 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2010-04-08  9:54 UTC (permalink / raw)
  To: kaber; +Cc: fw, netdev, johannes

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 08 Apr 2010 11:44:43 +0200

> Either the kernel or the userspace programs have to be updated
> either way.

The only case in which we only have to change one side is if we add
the full set of compat support to the kernel.

If we take any other option (new XFRM numbers and new datastructures,
or only convert sendmsg() to do compat translations), it requires both
the kernel and userspace to change.

And the currently existing 32-bit binaries don't work on 64-bit
kernels because of something that cannot be classified any other way
than as being a kernel bug.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-08  9:54           ` David Miller
@ 2010-04-08 11:24             ` Patrick McHardy
  0 siblings, 0 replies; 14+ messages in thread
From: Patrick McHardy @ 2010-04-08 11:24 UTC (permalink / raw)
  To: David Miller; +Cc: fw, netdev, johannes

David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Thu, 08 Apr 2010 11:44:43 +0200
> 
>> Either the kernel or the userspace programs have to be updated
>> either way.
> 
> The only case in which we only have to change one side is if we add
> the full set of compat support to the kernel.
> 
> If we take any other option (new XFRM numbers and new datastructures,
> or only convert sendmsg() to do compat translations), it requires both
> the kernel and userspace to change.

You're right of course.

> And the currently existing 32-bit binaries don't work on 64-bit
> kernels because of something that cannot be classified any other way
> than as being a kernel bug.

Agreed, but since its not a new bug, I think we have some flexibility
in how to fix it. In the wireless case we had no real choice but to
add the COMPAT_NETLINK thing, but its a bit sad to add more of this
crap to netlink.

Anyways, I guess there's no reason why we couldn't do both and
transistion to a fixed API in the long term.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support
  2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
                   ` (4 preceding siblings ...)
  2010-04-07 10:08 ` [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support David Miller
@ 2010-05-13  6:41 ` David Miller
  5 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2010-05-13  6:41 UTC (permalink / raw)
  To: fw; +Cc: netdev, johannes


I'm officially deferring this patch set for now, sorry.

I can't see applying this if it will fix only some subset of
known applications.  We need buy-in on the read/write vfs ops
override so we can do this properly.

Thanks for all of your efforts Florian.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2010-05-13  6:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-05 22:27 [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support Florian Westphal
2010-04-05 22:27 ` [PATCH 1/4] netlink: append NLMSG_DONE to compatskb, too Florian Westphal
2010-04-05 22:27 ` [PATCH 2/4] netlink: store MSG_CMSG_COMPAT flag in netlink_skb_parms Florian Westphal
2010-04-05 22:27 ` [PATCH 3/4] xfrm: split nlmsg allocation and data copying Florian Westphal
2010-04-05 22:27 ` [PATCH 4/4] xfrm: CONFIG_COMPAT support for x86 architecture Florian Westphal
2010-04-07 10:08 ` [PATCH v3 0/4] xfrm: add x86 CONFIG_COMPAT support David Miller
2010-04-07 13:35   ` Florian Westphal
2010-04-07 13:45     ` Patrick McHardy
2010-04-07 23:48       ` David Miller
2010-04-08  9:44         ` Patrick McHardy
2010-04-08  9:54           ` David Miller
2010-04-08 11:24             ` Patrick McHardy
2010-04-08  5:00     ` David Miller
2010-05-13  6:41 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.