diff mbox

[net-next] xfrm: Correctly parse netlink msg from 32bits ip command on 64bits host

Message ID 530C3B07.3090406@windriver.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

fan.du Feb. 25, 2014, 6:41 a.m. UTC
On 2014年02月20日 17:59, Steffen Klassert wrote:
> For now I think we should just refuse to do anything if someone tries
> to configure ipsec with 32 bit tools on a 64 bit machine.

I'm fine with your point, and it would be a good choice to inform user about
this behavior other than just creating non-working SA and SP for user.


 From 873812ec0fe8738f476de58a217e58ec47665180 Mon Sep 17 00:00:00 2001
From: Fan Du <fan.du@windriver.com>
Date: Tue, 25 Feb 2014 14:34:41 +0800
Subject: [PATCH net-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on
  64bits host

structure like xfrm_usersa_info or xfrm_userpolicy_info has different sizeof
when compiled as 32bits and 64bits due to not appending pack attribute in
their definition. This will result in broken SA and SP information when user
trying to configure them through netlink interface.

Before forging a compatibility layer like we have it for system calls to map
this correct. Inform user land about this situation instead of keeping silent,
then the upper test scripts could behave accordingly.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
  net/xfrm/xfrm_user.c |    4 ++++
  1 file changed, 4 insertions(+)

Comments

Steffen Klassert Feb. 25, 2014, 11:53 a.m. UTC | #1
On Tue, Feb 25, 2014 at 02:41:11PM +0800, Fan Du wrote:
> 
> 
> On 2014年02月20日 17:59, Steffen Klassert wrote:
> >For now I think we should just refuse to do anything if someone tries
> >to configure ipsec with 32 bit tools on a 64 bit machine.
> 
> I'm fine with your point, and it would be a good choice to inform user about
> this behavior other than just creating non-working SA and SP for user.
> 
> 
> From 873812ec0fe8738f476de58a217e58ec47665180 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Tue, 25 Feb 2014 14:34:41 +0800
> Subject: [PATCH net-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on
>  64bits host
> 
> structure like xfrm_usersa_info or xfrm_userpolicy_info has different sizeof
> when compiled as 32bits and 64bits due to not appending pack attribute in
> their definition. This will result in broken SA and SP information when user
> trying to configure them through netlink interface.
> 
> Before forging a compatibility layer like we have it for system calls to map
> this correct. Inform user land about this situation instead of keeping silent,
> then the upper test scripts could behave accordingly.
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>

I'm ok with your patch, but it does not apply to ipsec-next.
Please rebase to ipsec-next current and resend.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings Feb. 25, 2014, 5:15 p.m. UTC | #2
On Tue, 2014-02-25 at 14:41 +0800, Fan Du wrote:
> 
> On 2014年02月20日 17:59, Steffen Klassert wrote:
> > For now I think we should just refuse to do anything if someone tries
> > to configure ipsec with 32 bit tools on a 64 bit machine.
> 
> I'm fine with your point, and it would be a good choice to inform user about
> this behavior other than just creating non-working SA and SP for user.
> 
> 
>  From 873812ec0fe8738f476de58a217e58ec47665180 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Tue, 25 Feb 2014 14:34:41 +0800
> Subject: [PATCH net-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on
>   64bits host
> 
> structure like xfrm_usersa_info or xfrm_userpolicy_info has different sizeof
> when compiled as 32bits and 64bits due to not appending pack attribute in
> their definition. This will result in broken SA and SP information when user
> trying to configure them through netlink interface.
> 
> Before forging a compatibility layer like we have it for system calls to map
> this correct. Inform user land about this situation instead of keeping silent,
> then the upper test scripts could behave accordingly.
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---
>   net/xfrm/xfrm_user.c |    4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 1ae3ec7..0249712 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2347,6 +2347,10 @@ static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>   	const struct xfrm_link *link;
>   	int type, err;
> 
> +#ifdef CONFIG_COMPAT
> +	if (is_compat_task())
> +		return -EPERM;

I think this needs a log message, as it is not at all obvious that EPERM
means you ran a binary from the 'wrong' architecture.

Ben.

> +#endif
>   	type = nlh->nlmsg_type;
>   	if (type > XFRM_MSG_MAX)
>   		return -EINVAL;
Florian Westphal Feb. 25, 2014, 7:16 p.m. UTC | #3
Ben Hutchings <ben@decadent.org.uk> wrote:
> > On 2014年02月20日 17:59, Steffen Klassert wrote:
> > > For now I think we should just refuse to do anything if someone tries
> > > to configure ipsec with 32 bit tools on a 64 bit machine.
> > 
> > I'm fine with your point, and it would be a good choice to inform user about
> > this behavior other than just creating non-working SA and SP for user.

It might be better to try to revive
http://thread.gmane.org/gmane.linux.network/157118/focus=157122

which adds full compat support.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Feb. 27, 2014, 11:52 p.m. UTC | #4
From: Florian Westphal <fw@strlen.de>
Date: Tue, 25 Feb 2014 20:16:02 +0100

> Ben Hutchings <ben@decadent.org.uk> wrote:
>> > On 2014年02月20日 17:59, Steffen Klassert wrote:
>> > > For now I think we should just refuse to do anything if someone tries
>> > > to configure ipsec with 32 bit tools on a 64 bit machine.
>> > 
>> > I'm fine with your point, and it would be a good choice to inform user about
>> > this behavior other than just creating non-working SA and SP for user.
> 
> It might be better to try to revive
> http://thread.gmane.org/gmane.linux.network/157118/focus=157122
> 
> which adds full compat support.

Agreed.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 1ae3ec7..0249712 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2347,6 +2347,10 @@  static int xfrm_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
  	const struct xfrm_link *link;
  	int type, err;

+#ifdef CONFIG_COMPAT
+	if (is_compat_task())
+		return -EPERM;
+#endif
  	type = nlh->nlmsg_type;
  	if (type > XFRM_MSG_MAX)
  		return -EINVAL;