| Message ID | 1405303064-9102-7-git-send-email-jlayton@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jul 13, 2014 at 09:57:43PM -0400, Jeff Layton wrote: > Signed-off-by: Jeff Layton <jlayton@primarydata.com> > --- > net/sunrpc/auth_gss/gss_krb5_wrap.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c > index 42560e55d978..c5cc0270a334 100644 > --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c > +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c > @@ -201,9 +201,9 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset, > > msg_start = ptr + GSS_KRB5_TOK_HDR_LEN + kctx->gk5e->cksumlength; > > + *(__be16 *)(ptr + 2) = (__force __be16)cpu_to_le16(kctx->gk5e->signalg); This looks silly. This should be: *(__le16 *)(ptr + 2) = cpu_to_le16(kctx->gk5e->signalg); Maybe with a comment somewhere explaining why we're doing little endian encoding here if it's not obvious from the surrounding code. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 15 Jul 2014 10:04:42 -0700 Christoph Hellwig <hch@infradead.org> wrote: > On Sun, Jul 13, 2014 at 09:57:43PM -0400, Jeff Layton wrote: > > Signed-off-by: Jeff Layton <jlayton@primarydata.com> > > --- > > net/sunrpc/auth_gss/gss_krb5_wrap.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c > > index 42560e55d978..c5cc0270a334 100644 > > --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c > > +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c > > @@ -201,9 +201,9 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset, > > > > msg_start = ptr + GSS_KRB5_TOK_HDR_LEN + kctx->gk5e->cksumlength; > > > > + *(__be16 *)(ptr + 2) = (__force __be16)cpu_to_le16(kctx->gk5e->signalg); > > This looks silly. This should be: > > *(__le16 *)(ptr + 2) = cpu_to_le16(kctx->gk5e->signalg); > > Maybe with a comment somewhere explaining why we're doing little endian > encoding here if it's not obvious from the surrounding code. > The spec doesn't really define these as little-endian values, it's just an opaque series of bytes that the kernel implementation happens to handle as little-endian (see RFC 1964, section 1.2.1). Maybe I should reverse the bytes and we can just make that cpu_to_be16 instead? So the code is correct, AFAICT -- it's just odd...
On Tue, Jul 15, 2014 at 01:36:14PM -0400, Jeff Layton wrote: > > > > > > + *(__be16 *)(ptr + 2) = (__force __be16)cpu_to_le16(kctx->gk5e->signalg); > > > > This looks silly. This should be: > > > > *(__le16 *)(ptr + 2) = cpu_to_le16(kctx->gk5e->signalg); > > > > Maybe with a comment somewhere explaining why we're doing little endian > > encoding here if it's not obvious from the surrounding code. > > > > The spec doesn't really define these as little-endian values, it's just > an opaque series of bytes that the kernel implementation happens to > handle as little-endian (see RFC 1964, section 1.2.1). Maybe I should > reverse the bytes and we can just make that cpu_to_be16 instead? Sounds okay to me. > So the code is correct, AFAICT -- it's just odd... It might be correct, but with the added cast it's simply too ugly to live. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c index 42560e55d978..c5cc0270a334 100644 --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c @@ -201,9 +201,9 @@ gss_wrap_kerberos_v1(struct krb5_ctx *kctx, int offset, msg_start = ptr + GSS_KRB5_TOK_HDR_LEN + kctx->gk5e->cksumlength; - *(__be16 *)(ptr + 2) = cpu_to_le16(kctx->gk5e->signalg); + *(__be16 *)(ptr + 2) = (__force __be16)cpu_to_le16(kctx->gk5e->signalg); memset(ptr + 4, 0xff, 4); - *(__be16 *)(ptr + 4) = cpu_to_le16(kctx->gk5e->sealalg); + *(__be16 *)(ptr + 4) = (__force __be16)cpu_to_le16(kctx->gk5e->sealalg); gss_krb5_make_confounder(msg_start, conflen); @@ -438,7 +438,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32 offset, u8 *ptr, *plainhdr; s32 now; u8 flags = 0x00; - __be16 *be16ptr, ec = 0; + __be16 *be16ptr; __be64 *be64ptr; u32 err; @@ -468,7 +468,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32 offset, be16ptr = (__be16 *)ptr; blocksize = crypto_blkcipher_blocksize(kctx->acceptor_enc); - *be16ptr++ = cpu_to_be16(ec); + *be16ptr++ = cpu_to_be16(0); /* "inner" token header always uses 0 for RRC */ *be16ptr++ = cpu_to_be16(0); @@ -477,7 +477,7 @@ gss_wrap_kerberos_v2(struct krb5_ctx *kctx, u32 offset, *be64ptr = cpu_to_be64(kctx->seq_send64++); spin_unlock(&krb5_seq_lock); - err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, ec, pages); + err = (*kctx->gk5e->encrypt_v2)(kctx, offset, buf, 0, pages); if (err) return err;
Signed-off-by: Jeff Layton <jlayton@primarydata.com> --- net/sunrpc/auth_gss/gss_krb5_wrap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)