Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1251090 > unrolled thread

[PATCH 0/2] xfrm/crypto: unaligned access fixes

Started bySowmini Varadhan <sowmini.varadhan@oracle.com>
First post2015-10-19 23:30 +0200
Last post2015-10-21 15:00 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] xfrm/crypto: unaligned access fixes Sowmini Varadhan <sowmini.varadhan@oracle.com> - 2015-10-19 23:30 +0200
    [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA Sowmini Varadhan <sowmini.varadhan@oracle.com> - 2015-10-19 23:30 +0200
      Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA Steffen Klassert <steffen.klassert@secunet.com> - 2015-10-21 09:00 +0200
        Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA Sowmini Varadhan <sowmini.varadhan@oracle.com> - 2015-10-21 13:00 +0200
          Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA Sowmini Varadhan <sowmini.varadhan@oracle.com> - 2015-10-21 14:40 +0200
            Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA David Miller <davem@davemloft.net> - 2015-10-21 15:10 +0200
              Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA Sowmini Varadhan <sowmini.varadhan@oracle.com> - 2015-10-21 15:20 +0200
          Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA David Miller <davem@davemloft.net> - 2015-10-21 15:10 +0200
        Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in  xfrm_notify_sa() for DELSA David Miller <davem@davemloft.net> - 2015-10-21 15:00 +0200

#1251090 — [PATCH 0/2] xfrm/crypto: unaligned access fixes

FromSowmini Varadhan <sowmini.varadhan@oracle.com>
Date2015-10-19 23:30 +0200
Subject[PATCH 0/2] xfrm/crypto: unaligned access fixes
Message-ID<qlqad-8it-7@gated-at.bofh.it>
A two-part patchset that fixes some "unaligned access" warnings
that showed up my sparc test machines with ipsec set up.


Sowmini Varadhan (2):
  crypto/x509: Fix unaligned access in x509_get_sig_params()
  Fix unaligned access in xfrm_notify_sa() for DELSA

 crypto/asymmetric_keys/x509_public_key.c |    5 +++--
 net/xfrm/xfrm_user.c                     |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1251093 — [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromSowmini Varadhan <sowmini.varadhan@oracle.com>
Date2015-10-19 23:30 +0200
Subject[PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qlqae-8it-43@gated-at.bofh.it>
In reply to#1251090
On sparc, deleting established SAs (e.g., by restarting ipsec
at the peer) results in unaligned access messages via
xfrm_del_sa -> km_state_notify -> xfrm_send_state_notify().
Use an aligned pointer to xfrm_usersa_info for this case.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 net/xfrm/xfrm_user.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index a8de9e3..158ef4a 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
 		if (attr == NULL)
 			goto out_free_skb;
 
-		p = nla_data(attr);
+		p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
 	}
 	err = copy_to_user_state_extra(x, p, skb);
 	if (err)
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252504 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromSteffen Klassert <steffen.klassert@secunet.com>
Date2015-10-21 09:00 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qlVxn-3Et-1@gated-at.bofh.it>
In reply to#1251093
On Mon, Oct 19, 2015 at 05:23:29PM -0400, Sowmini Varadhan wrote:
> On sparc, deleting established SAs (e.g., by restarting ipsec
> at the peer) results in unaligned access messages via
> xfrm_del_sa -> km_state_notify -> xfrm_send_state_notify().
> Use an aligned pointer to xfrm_usersa_info for this case.
> 
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
>  net/xfrm/xfrm_user.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index a8de9e3..158ef4a 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
>  		if (attr == NULL)
>  			goto out_free_skb;
>  
> -		p = nla_data(attr);
> +		p = PTR_ALIGN(nla_data(attr), __alignof__(*p));

Hm, this breaks userspace notifications on 64-bit systems.
Userspace expects this to be aligned to 4, with your patch
it is aligned to 8 on 64-bit.

Without your patch I get the correct notification when deleting a SA:

ip x m

Deleted src 172.16.0.2 dst 172.16.0.1
        proto esp spi 0x00000002 reqid 2 mode tunnel
        replay-window 32
        auth-trunc hmac(sha1) 0x31323334353637383930 96
        enc cbc(aes) 0x31323334353637383930313233343536
        sel src 10.0.0.0/24 dst 192.168.0.0/24

With your patch I get for the same SA:

ip x m

Deleted src 50.0.0.0 dst 0.0.0.0
        proto 0 reqid 0 mode transport
        replay-window 0 flag decap-dscp
        auth-trunc hmac(sha1) 0x31323334353637383930 96
        enc cbc(aes) 0x31323334353637383930313233343536
        sel src 0.0.0.0/0 dst 0.234.255.255/0 proto igmp

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252716 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromSowmini Varadhan <sowmini.varadhan@oracle.com>
Date2015-10-21 13:00 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qlZhE-FD-13@gated-at.bofh.it>
In reply to#1252504
On (10/21/15 08:57), Steffen Klassert wrote:
> > --- a/net/xfrm/xfrm_user.c
> > +++ b/net/xfrm/xfrm_user.c
> > @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
> >  		if (attr == NULL)
> >  			goto out_free_skb;
> >  
> > -		p = nla_data(attr);
> > +		p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
> 
> Hm, this breaks userspace notifications on 64-bit systems.
> Userspace expects this to be aligned to 4, with your patch
> it is aligned to 8 on 64-bit.
> 
> Without your patch I get the correct notification when deleting a SA:
> 

But __alignof__(*p) is 8 on sparc, and without the patch I get
all types of unaligned access. So what do you suggest as the fix?

(and openswan/pluto dont flag any errors with the patch, which is
a separate bug).

--Sowmini

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252782 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromSowmini Varadhan <sowmini.varadhan@oracle.com>
Date2015-10-21 14:40 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qm0Qp-33U-7@gated-at.bofh.it>
In reply to#1252716
On (10/21/15 06:54), Sowmini Varadhan wrote:
> But __alignof__(*p) is 8 on sparc, and without the patch I get
> all types of unaligned access. So what do you suggest as the fix?

Even though the alignment is, in fact, 8 (and that comes from
struct xfrm_lifetime_cfg), if uspace is firmly attached to the 4 byte
alignment, I think we can retain that behavior and still avoid
unaligned access in the kernel with the following (admittedly ugly hack).
Can you please take a look? I tested it with 'ip x m' and a transport 
mode tunnel on my sparc.


diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 158ef4a..ca4e7f0 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2620,7 +2620,7 @@ static inline size_t xfrm_sa_len(struct xfrm_state *x)
 static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
 {
        struct net *net = xs_net(x);
-       struct xfrm_usersa_info *p;
+       struct xfrm_usersa_info *p, tmp;
        struct xfrm_usersa_id *id;
        struct nlmsghdr *nlh;
        struct sk_buff *skb;
@@ -2659,11 +2659,16 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
                if (attr == NULL)
                        goto out_free_skb;
 
-               p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
+               p = nla_data(attr);
+               err = copy_to_user_state_extra(x, &tmp, skb);
+               if (err)
+                       goto out_free_skb;
+               memcpy((u8 *)p, &tmp, sizeof(tmp));
+       } else {
+               err = copy_to_user_state_extra(x, p, skb);
+               if (err)
+                       goto out_free_skb;
        }
-       err = copy_to_user_state_extra(x, p, skb);
-       if (err)
-               goto out_free_skb;
 
        nlmsg_end(skb, nlh);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252797 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromDavid Miller <davem@davemloft.net>
Date2015-10-21 15:10 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qm1js-3S5-17@gated-at.bofh.it>
In reply to#1252782
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Wed, 21 Oct 2015 08:36:28 -0400

> +               memcpy((u8 *)p, &tmp, sizeof(tmp));

memcpy() _never_ works for avoiding unaligned accessed.

I repeat, no matter what you do, no matter what kinds of casts or
fancy typing you use, memcpy() _never_ works for this purpose.

The compiler knows that the pointer you are using is supposed to be at
least 8 byte aligned, it can look through the cast and that's
completely legitimate for it to do.

So it can legitimately inline emit loads and stores to implement the
memcpy() and those will still get the unaligned accesses.

There is one and only one portable way to access unaligned data,
and that is with the get_unaligned() and put_unaligned() helpers.

Userland must do something similar to deal with this situation
as well.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252801 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromSowmini Varadhan <sowmini.varadhan@oracle.com>
Date2015-10-21 15:20 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qm1t8-43q-7@gated-at.bofh.it>
In reply to#1252797
On (10/21/15 06:22), David Miller wrote:
> memcpy() _never_ works for avoiding unaligned accessed.
> 
> I repeat, no matter what you do, no matter what kinds of casts or
> fancy typing you use, memcpy() _never_ works for this purpose.
  :
> There is one and only one portable way to access unaligned data,
> and that is with the get_unaligned() and put_unaligned() helpers.

ok. I'll fix it up to use the *_unaligned functions and resend this 
out later today.

--Sowmini

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252799 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromDavid Miller <davem@davemloft.net>
Date2015-10-21 15:10 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qm1js-3S5-19@gated-at.bofh.it>
In reply to#1252716
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Date: Wed, 21 Oct 2015 06:54:42 -0400

> On (10/21/15 08:57), Steffen Klassert wrote:
>> > --- a/net/xfrm/xfrm_user.c
>> > +++ b/net/xfrm/xfrm_user.c
>> > @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
>> >  		if (attr == NULL)
>> >  			goto out_free_skb;
>> >  
>> > -		p = nla_data(attr);
>> > +		p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
>> 
>> Hm, this breaks userspace notifications on 64-bit systems.
>> Userspace expects this to be aligned to 4, with your patch
>> it is aligned to 8 on 64-bit.
>> 
>> Without your patch I get the correct notification when deleting a SA:
>> 
> 
> But __alignof__(*p) is 8 on sparc, and without the patch I get
> all types of unaligned access. So what do you suggest as the fix?

The accesses have to be done using something like get_unaligned() and
put_unaligned().

Sorry, but the protocol is set in stone and this is unfortunately how
it is.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1252791 — Re: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA

FromDavid Miller <davem@davemloft.net>
Date2015-10-21 15:00 +0200
SubjectRe: [PATCH net-next 2/2] xfrm: Fix unaligned access in xfrm_notify_sa() for DELSA
Message-ID<qm19M-3rA-15@gated-at.bofh.it>
In reply to#1252504
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed, 21 Oct 2015 08:57:04 +0200

> On Mon, Oct 19, 2015 at 05:23:29PM -0400, Sowmini Varadhan wrote:
>> On sparc, deleting established SAs (e.g., by restarting ipsec
>> at the peer) results in unaligned access messages via
>> xfrm_del_sa -> km_state_notify -> xfrm_send_state_notify().
>> Use an aligned pointer to xfrm_usersa_info for this case.
>> 
>> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
>> ---
>>  net/xfrm/xfrm_user.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
>> index a8de9e3..158ef4a 100644
>> --- a/net/xfrm/xfrm_user.c
>> +++ b/net/xfrm/xfrm_user.c
>> @@ -2659,7 +2659,7 @@ static int xfrm_notify_sa(struct xfrm_state *x, const struct km_event *c)
>>  		if (attr == NULL)
>>  			goto out_free_skb;
>>  
>> -		p = nla_data(attr);
>> +		p = PTR_ALIGN(nla_data(attr), __alignof__(*p));
> 
> Hm, this breaks userspace notifications on 64-bit systems.
> Userspace expects this to be aligned to 4, with your patch
> it is aligned to 8 on 64-bit.

That's correct, netlink attributes are fundamentally only 4 byte
aligned and this cannot be changed.  nla_data() is exactly
where the attribute must be placed, aligned or not.

The only workaround is, when designing netlink attributes.  Various
netlink libraries have workarounds for accessing, for example, 64-bit
stats which are going to be unaligned in netlink messages.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web