From: Herbert Xu <herbert@gondor.apana.org.au>
To: Ayush Sawal <ayush.sawal@chelsio.com>
Cc: viro@zeniv.linux.org.uk, linux-crypto@vger.kernel.org,
linux-kernel@vger.kernel.org, vinay.yadav@chelsio.com
Subject: Re: [RFC][PATCH] almost certain bug in drivers/crypto/chelsio/chcr_algo.c:create_authenc_wr()
Date: Sat, 22 Feb 2020 12:44:05 +1100 [thread overview]
Message-ID: <20200222014405.GA19322@gondor.apana.org.au> (raw)
In-Reply-To: <db4ee9c7-400e-5932-8708-581d91b38385@chelsio.com>
On Fri, Feb 21, 2020 at 10:47:01AM +0530, Ayush Sawal wrote:
> On 2/15/2020 11:45 AM, Al Viro wrote:
>
> >
> > kctx_len = (ntohl(KEY_CONTEXT_CTX_LEN_V(aeadctx->key_ctx_hdr))
> > << 4)
> > - sizeof(chcr_req->key_ctx);
> > can't possibly be endian-safe. Look: ->key_ctx_hdr is __be32. And
> > KEY_CONTEXT_CTX_LEN_V is "shift up by 24 bits". On little-endian hosts it
> > sees
> > b0 b1 b2 b3
> > in memory, inteprets that into b0 + (b1 << 8) + (b2 << 16) + (b3 << 24),
> > shifts up by 24, resulting in b0 << 24, does ntohl (byteswap on l-e),
> > gets b0 and shifts that up by 4. So we get b0 * 16 - sizeof(...).
> >
> > Sounds reasonable, but on b-e we get
> > b3 + (b2 << 8) + (b1 << 16) + (b0 << 24), shift up by 24,
> > yielding b3 << 24, do ntohl (no-op on b-e) and then shift up by 4.
> > Resulting in b3 << 28 - sizeof(...), i.e. slightly under b3 * 256M.
> >
> > Then we increase it some more and pass to alloc_skb() as size.
> > Somehow I doubt that we really want a quarter-gigabyte skb allocation
> > here...
> >
> > Note that when you are building those values in
> > #define FILL_KEY_CTX_HDR(ck_size, mk_size, d_ck, opad, ctx_len) \
> > htonl(KEY_CONTEXT_VALID_V(1) | \
> > KEY_CONTEXT_CK_SIZE_V((ck_size)) | \
> > KEY_CONTEXT_MK_SIZE_V(mk_size) | \
> > KEY_CONTEXT_DUAL_CK_V((d_ck)) | \
> > KEY_CONTEXT_OPAD_PRESENT_V((opad)) | \
> > KEY_CONTEXT_SALT_PRESENT_V(1) | \
> > KEY_CONTEXT_CTX_LEN_V((ctx_len)))
> > ctx_len ends up in the first octet (i.e. b0 in the above), which
> > matches the current behaviour on l-e. If that's the intent, this
> > thing should've been
> > kctx_len = (KEY_CONTEXT_CTX_LEN_G(ntohl(aeadctx->key_ctx_hdr))
> > << 4)
> > - sizeof(chcr_req->key_ctx);
> > instead - fetch after ntohl() we get (b0 << 24) + (b1 << 16) + (b2 << 8)
> > + b3,
> > shift it down by 24 (b0), resuling in b0 * 16 - sizeof(...) both on l-e
> > and
> > on b-e.
> >
> > PS: when sparse warns you about endianness problems, it might be worth
> > checking
> > if there really is something wrong. And I don't mean "slap __force cast
> > on it"...
> >
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk
> > <mailto:viro@zeniv.linux.org.uk>>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
prev parent reply other threads:[~2020-02-22 1:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-15 6:14 [RFC][PATCH] almost certain bug in drivers/crypto/chelsio/chcr_algo.c:create_authenc_wr() Al Viro
2020-02-15 6:29 ` Al Viro
[not found] ` <CAEopUdxRUoMo+uGgiFLWz8NsM1eL7CnkV7gY5PypxrG_nzhNWw@mail.gmail.com>
2020-02-21 5:17 ` Ayush Sawal
2020-02-22 1:44 ` Herbert Xu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200222014405.GA19322@gondor.apana.org.au \
--to=herbert@gondor.apana.org.au \
--cc=ayush.sawal@chelsio.com \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vinay.yadav@chelsio.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.