Unaligned access in gss_{get,verify}_mic_v2() on sparc64

Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[James Ettle]

Hello,

I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
seeing a lot of messages like:

sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]

I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
the suspicious line is:

	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);

krb5_hdr is void*, but comes from a u16 so won't generally have __be64
alignment. As an experiment I added local variable

	__be64 seq_send_be64;

and replaced the cpu_to_be64 line with:

	seq_send_be64 = cpu_to_be64(seq_send);
	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);

There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
there's a line

	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)

but ptr is a u8*. For this I added local variable

	__be16 data_be16;

and replaced the above if() with

	memcpy((void *) &data_be16, (char *) ptr, 2);
	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)

I've not seen any misalignment complaints yet with this.

I apologise for not sending this in the form of a patch, but this is
only a sketch solution. I'm not a kernel hacker and I'm sure someone
else will make a proper job of it!

Thanks,

James.

Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[James Ettle]

Hello,

I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
seeing a lot of messages like:

sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]

I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
the suspicious line is:

	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);

krb5_hdr is void*, but comes from a u16 so won't generally have __be64
alignment. As an experiment I added local variable

	__be64 seq_send_be64;

and replaced the cpu_to_be64 line with:

	seq_send_be64 = cpu_to_be64(seq_send);
	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);

There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
there's a line

	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)

but ptr is a u8*. For this I added local variable

	__be16 data_be16;

and replaced the above if() with

	memcpy((void *) &data_be16, (char *) ptr, 2);
	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)

I've not seen any misalignment complaints yet with this.

I apologise for not sending this in the form of a patch, but this is
only a sketch solution. I'm not a kernel hacker and I'm sure someone
else will make a proper job of it!

Thanks,

James.

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[J. Bruce Fields]

On Sat, Dec 02, 2017 at 11:28:18PM +0000, James Ettle wrote:
> I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
> seeing a lot of messages like:
> 
> sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
> gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]
> 
> I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
> the suspicious line is:
> 
> 	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
> 
> krb5_hdr is void*, but comes from a u16 so won't generally have __be64
> alignment. As an experiment I added local variable
> 
> 	__be64 seq_send_be64;
> 
> and replaced the cpu_to_be64 line with:
> 
> 	seq_send_be64 = cpu_to_be64(seq_send);
> 	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
> 
> There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
> there's a line
> 
> 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
> 
> but ptr is a u8*. For this I added local variable
> 
> 	__be16 data_be16;
> 
> and replaced the above if() with
> 
> 	memcpy((void *) &data_be16, (char *) ptr, 2);
> 	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)
> 
> I've not seen any misalignment complaints yet with this.
> 
> I apologise for not sending this in the form of a patch, but this is
> only a sketch solution. I'm not a kernel hacker and I'm sure someone
> else will make a proper job of it!

Probably so.  But it might get done sooner if you do it.  There's the
added benefit that you can test the exact patch that gets applied.  You
can just:

	git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
	<make your changes>
	git commit -a
	<write a changelog>

then send us the output of "git show".

--b.

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[J. Bruce Fields]

On Sat, Dec 02, 2017 at 11:28:18PM +0000, James Ettle wrote:
> I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
> seeing a lot of messages like:
> 
> sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
> gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]
> 
> I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
> the suspicious line is:
> 
> 	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
> 
> krb5_hdr is void*, but comes from a u16 so won't generally have __be64
> alignment. As an experiment I added local variable
> 
> 	__be64 seq_send_be64;
> 
> and replaced the cpu_to_be64 line with:
> 
> 	seq_send_be64 = cpu_to_be64(seq_send);
> 	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
> 
> There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
> there's a line
> 
> 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
> 
> but ptr is a u8*. For this I added local variable
> 
> 	__be16 data_be16;
> 
> and replaced the above if() with
> 
> 	memcpy((void *) &data_be16, (char *) ptr, 2);
> 	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)
> 
> I've not seen any misalignment complaints yet with this.
> 
> I apologise for not sending this in the form of a patch, but this is
> only a sketch solution. I'm not a kernel hacker and I'm sure someone
> else will make a proper job of it!

Probably so.  But it might get done sooner if you do it.  There's the
added benefit that you can test the exact patch that gets applied.  You
can just:

	git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
	<make your changes>
	git commit -a
	<write a changelog>

then send us the output of "git show".

--b.

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[James Ettle]

Patch from git as instructed:


commit a90324ca784dea5a7259a2672c24626f5c03f576
Author: James Ettle <james@ettle.org.uk>
Date:   Thu Dec 7 00:50:28 2017 +0000

    Fix unaligned access on sparc64.

diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c b/net/sunrpc/auth_gss/gss_krb5_seal.c
index 1d74d653e6c0..94a2b3f082a8 100644
--- a/net/sunrpc/auth_gss/gss_krb5_seal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
@@ -177,6 +177,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
 	u64 seq_send;
 	u8 *cksumkey;
 	unsigned int cksum_usage;
+	__be64 seq_send_be64;

 	dprintk("RPC:       %s\n", __func__);

@@ -187,7 +188,9 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
 	spin_lock(&krb5_seq_lock);
 	seq_send = ctx->seq_send64++;
 	spin_unlock(&krb5_seq_lock);
-	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
+
+	seq_send_be64 = cpu_to_be64(seq_send);
+	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);

 	if (ctx->initiate) {
 		cksumkey = ctx->initiator_sign;
diff --git a/net/sunrpc/auth_gss/gss_krb5_unseal.c b/net/sunrpc/auth_gss/gss_krb5_unseal.c
index dcf9515d9aef..8ea6e30d6f3f 100644
--- a/net/sunrpc/auth_gss/gss_krb5_unseal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_unseal.c
@@ -155,10 +155,12 @@ gss_verify_mic_v2(struct krb5_ctx *ctx,
 	u8 flags;
 	int i;
 	unsigned int cksum_usage;
-
+	__be16 be16_ptr;
+	
 	dprintk("RPC:       %s\n", __func__);

-	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
+	memcpy(&be16_ptr, (char *) ptr, 2);
+	if (be16_to_cpu(be16_ptr) != KG2_TOK_MIC)
 		return GSS_S_DEFECTIVE_TOKEN;

 	flags = ptr[2];



On 06/12/17 19:09, J. Bruce Fields wrote:
> On Sat, Dec 02, 2017 at 11:28:18PM +0000, James Ettle wrote:
>> I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
>> seeing a lot of messages like:
>>
>> sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
>> gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]
>>
>> I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
>> the suspicious line is:
>>
>> 	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
>>
>> krb5_hdr is void*, but comes from a u16 so won't generally have __be64
>> alignment. As an experiment I added local variable
>>
>> 	__be64 seq_send_be64;
>>
>> and replaced the cpu_to_be64 line with:
>>
>> 	seq_send_be64 = cpu_to_be64(seq_send);
>> 	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
>>
>> There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
>> there's a line
>>
>> 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
>>
>> but ptr is a u8*. For this I added local variable
>>
>> 	__be16 data_be16;
>>
>> and replaced the above if() with
>>
>> 	memcpy((void *) &data_be16, (char *) ptr, 2);
>> 	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)
>>
>> I've not seen any misalignment complaints yet with this.
>>
>> I apologise for not sending this in the form of a patch, but this is
>> only a sketch solution. I'm not a kernel hacker and I'm sure someone
>> else will make a proper job of it!
> 
> Probably so.  But it might get done sooner if you do it.  There's the
> added benefit that you can test the exact patch that gets applied.  You
> can just:
> 
> 	git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> 	<make your changes>
> 	git commit -a
> 	<write a changelog>
> 
> then send us the output of "git show".
> 
> --b.
> 

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[James Ettle]

Patch from git as instructed:


commit a90324ca784dea5a7259a2672c24626f5c03f576
Author: James Ettle <james@ettle.org.uk>
Date:   Thu Dec 7 00:50:28 2017 +0000

    Fix unaligned access on sparc64.

diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c b/net/sunrpc/auth_gss/gss_krb5_seal.c
index 1d74d653e6c0..94a2b3f082a8 100644
--- a/net/sunrpc/auth_gss/gss_krb5_seal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
@@ -177,6 +177,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
 	u64 seq_send;
 	u8 *cksumkey;
 	unsigned int cksum_usage;
+	__be64 seq_send_be64;

 	dprintk("RPC:       %s\n", __func__);

@@ -187,7 +188,9 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
 	spin_lock(&krb5_seq_lock);
 	seq_send = ctx->seq_send64++;
 	spin_unlock(&krb5_seq_lock);
-	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
+
+	seq_send_be64 = cpu_to_be64(seq_send);
+	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);

 	if (ctx->initiate) {
 		cksumkey = ctx->initiator_sign;
diff --git a/net/sunrpc/auth_gss/gss_krb5_unseal.c b/net/sunrpc/auth_gss/gss_krb5_unseal.c
index dcf9515d9aef..8ea6e30d6f3f 100644
--- a/net/sunrpc/auth_gss/gss_krb5_unseal.c
+++ b/net/sunrpc/auth_gss/gss_krb5_unseal.c
@@ -155,10 +155,12 @@ gss_verify_mic_v2(struct krb5_ctx *ctx,
 	u8 flags;
 	int i;
 	unsigned int cksum_usage;
-
+	__be16 be16_ptr;
+	
 	dprintk("RPC:       %s\n", __func__);

-	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
+	memcpy(&be16_ptr, (char *) ptr, 2);
+	if (be16_to_cpu(be16_ptr) != KG2_TOK_MIC)
 		return GSS_S_DEFECTIVE_TOKEN;

 	flags = ptr[2];



On 06/12/17 19:09, J. Bruce Fields wrote:
> On Sat, Dec 02, 2017 at 11:28:18PM +0000, James Ettle wrote:
>> I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
>> seeing a lot of messages like:
>>
>> sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
>> gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]
>>
>> I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
>> the suspicious line is:
>>
>> 	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
>>
>> krb5_hdr is void*, but comes from a u16 so won't generally have __be64
>> alignment. As an experiment I added local variable
>>
>> 	__be64 seq_send_be64;
>>
>> and replaced the cpu_to_be64 line with:
>>
>> 	seq_send_be64 = cpu_to_be64(seq_send);
>> 	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
>>
>> There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
>> there's a line
>>
>> 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
>>
>> but ptr is a u8*. For this I added local variable
>>
>> 	__be16 data_be16;
>>
>> and replaced the above if() with
>>
>> 	memcpy((void *) &data_be16, (char *) ptr, 2);
>> 	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)
>>
>> I've not seen any misalignment complaints yet with this.
>>
>> I apologise for not sending this in the form of a patch, but this is
>> only a sketch solution. I'm not a kernel hacker and I'm sure someone
>> else will make a proper job of it!
> 
> Probably so.  But it might get done sooner if you do it.  There's the
> added benefit that you can test the exact patch that gets applied.  You
> can just:
> 
> 	git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
> 	<make your changes>
> 	git commit -a
> 	<write a changelog>
> 
> then send us the output of "git show".
> 
> --b.
> 

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[James Ettle]

Hi,

Just a polite prod in case it's fallen off the radar. The patch in this thread still applies cleanly to 4.14.15.

Thanks,
James.

On 07/12/17 01:42, James Ettle wrote:
> Patch from git as instructed:
> 
> 
> commit a90324ca784dea5a7259a2672c24626f5c03f576
> Author: James Ettle <james@ettle.org.uk>
> Date:   Thu Dec 7 00:50:28 2017 +0000
> 
>     Fix unaligned access on sparc64.
> 
> diff --git a/net/sunrpc/auth_gss/gss_krb5_seal.c b/net/sunrpc/auth_gss/gss_krb5_seal.c
> index 1d74d653e6c0..94a2b3f082a8 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_seal.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_seal.c
> @@ -177,6 +177,7 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
>  	u64 seq_send;
>  	u8 *cksumkey;
>  	unsigned int cksum_usage;
> +	__be64 seq_send_be64;
> 
>  	dprintk("RPC:       %s\n", __func__);
> 
> @@ -187,7 +188,9 @@ gss_get_mic_v2(struct krb5_ctx *ctx, struct xdr_buf *text,
>  	spin_lock(&krb5_seq_lock);
>  	seq_send = ctx->seq_send64++;
>  	spin_unlock(&krb5_seq_lock);
> -	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
> +
> +	seq_send_be64 = cpu_to_be64(seq_send);
> +	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
> 
>  	if (ctx->initiate) {
>  		cksumkey = ctx->initiator_sign;
> diff --git a/net/sunrpc/auth_gss/gss_krb5_unseal.c b/net/sunrpc/auth_gss/gss_krb5_unseal.c
> index dcf9515d9aef..8ea6e30d6f3f 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_unseal.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_unseal.c
> @@ -155,10 +155,12 @@ gss_verify_mic_v2(struct krb5_ctx *ctx,
>  	u8 flags;
>  	int i;
>  	unsigned int cksum_usage;
> -
> +	__be16 be16_ptr;
> +	
>  	dprintk("RPC:       %s\n", __func__);
> 
> -	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
> +	memcpy(&be16_ptr, (char *) ptr, 2);
> +	if (be16_to_cpu(be16_ptr) != KG2_TOK_MIC)
>  		return GSS_S_DEFECTIVE_TOKEN;
> 
>  	flags = ptr[2];
> 
> 
> 
> On 06/12/17 19:09, J. Bruce Fields wrote:
>> On Sat, Dec 02, 2017 at 11:28:18PM +0000, James Ettle wrote:
>>> I've been using nfs4 with krb5 on sparc64 with kernel 4.13 and 4.14 and
>>> seeing a lot of messages like:
>>>
>>> sparky kernel: [  105.262927] Kernel unaligned access at TPC[10afb234]
>>> gss_get_mic_kerberos+0xd4/0x360 [rpcsec_gss_krb5]
>>>
>>> I've traced this down to gss_get_mic_v2() in gss_krb5_seal.c. I think
>>> the suspicious line is:
>>>
>>> 	*((__be64 *)(krb5_hdr + 8)) = cpu_to_be64(seq_send);
>>>
>>> krb5_hdr is void*, but comes from a u16 so won't generally have __be64
>>> alignment. As an experiment I added local variable
>>>
>>> 	__be64 seq_send_be64;
>>>
>>> and replaced the cpu_to_be64 line with:
>>>
>>> 	seq_send_be64 = cpu_to_be64(seq_send);
>>> 	memcpy(krb5_hdr + 8, (char *) &seq_send_be64, 8);
>>>
>>> There's another one in gss_verify_mic_v2() in gss_krb5_unseal.c. Here
>>> there's a line
>>>
>>> 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_MIC)
>>>
>>> but ptr is a u8*. For this I added local variable
>>>
>>> 	__be16 data_be16;
>>>
>>> and replaced the above if() with
>>>
>>> 	memcpy((void *) &data_be16, (char *) ptr, 2);
>>> 	if (be16_to_cpu(data_be16) != KG2_TOK_MIC)
>>>
>>> I've not seen any misalignment complaints yet with this.
>>>
>>> I apologise for not sending this in the form of a patch, but this is
>>> only a sketch solution. I'm not a kernel hacker and I'm sure someone
>>> else will make a proper job of it!
>>
>> Probably so.  But it might get done sooner if you do it.  There's the
>> added benefit that you can test the exact patch that gets applied.  You
>> can just:
>>
>> 	git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
>> 	<make your changes>
>> 	git commit -a
>> 	<write a changelog>
>>
>> then send us the output of "git show".
>>
>> --b.
>>

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[Sam Ravnborg]

On Sun, Jan 28, 2018 at 04:34:57PM +0000, James Ettle wrote:
> Hi,
> 
> Just a polite prod in case it's fallen off the radar. The patch in this thread still applies cleanly to 4.14.15.
I would recommend you to send this as a new mail - with a proper [PATCH] in the title.
Otherwise it may not be picked up.

Posting a patch in the midle of a thread is no-go.

	Sam

Re: Unaligned access in gss_{get,verify}_mic_v2() on sparc64

[Sam Ravnborg]

On Sun, Jan 28, 2018 at 04:34:57PM +0000, James Ettle wrote:
> Hi,
> 
> Just a polite prod in case it's fallen off the radar. The patch in this thread still applies cleanly to 4.14.15.
I would recommend you to send this as a new mail - with a proper [PATCH] in the title.
Otherwise it may not be picked up.

Posting a patch in the midle of a thread is no-go.

	Sam