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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).