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.
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.
Patch from git as instructed:
commit a90324ca784dea5a7259a2672c24626f5c03f576
Author: James Ettle <[email protected]>
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.
>
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 <[email protected]>
> 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.
>>
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