From: Ard Biesheuvel <ardb@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
Herbert Xu <herbert@gondor.apana.org.au>,
Will Deacon <will@kernel.org>,
Android Kernel Team <kernel-team@android.com>
Subject: Re: [PATCH v3 1/7] crypto: handle zero sized AEAD inputs correctly
Date: Wed, 12 May 2021 23:24:09 +0200 [thread overview]
Message-ID: <CAMj1kXHofDrzEs4qc8VNCLpyL-Hc4PSg-JXKTckJvfD6qoK78Q@mail.gmail.com> (raw)
In-Reply-To: <YJw01Z3oxwY5Sfpa@gmail.com>
On Wed, 12 May 2021 at 22:04, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, May 12, 2021 at 08:44:33PM +0200, Ard Biesheuvel wrote:
> > There are corner cases where skcipher_walk_aead_[en|de]crypt() may be
> > invoked with a zero sized input, which is not rejected by the walker
> > code, but results in the skcipher_walk structure to not be fully
> > initialized. This will leave stale values in its page and buffer
> > members, which will be subsequently passed to kfree() or free_page() by
> > skcipher_walk_done(), resulting in a crash if those routines fail to
> > identify them as in valid inputs.
> >
> > Fix this by setting page and buffer to NULL even if the size of the
> > input is zero.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Is this fixing an existing bug, or only a bug that got exposed by this patchset?
> It would be helpful to make that clear (and if it fixes an existing bug, include
> a Fixes tag).
>
The CCM change in the last patch uncovers this issue, and I don't
think it is likely we would ever hit it anywhere else.
> Also, skcipher_walk_virt() doesn't set page and buffer to NULL, as it is
> currently expected that skcipher_walk_done() is only called when
> walk.nbytes != 0. Is something different for skcipher_walk_aead_[en|de]crypt()?
>
The difference is that zero sized inputs never make sense for
skciphers, but for AEADs, they could occur, even if they are uncommon
(the AEAD could have associated data only, and no plain/ciphertext)
But in the general case, I would assume that skcipher_walk_done() can
be called on a walk that was successfully started with
skcipher_walk_virt() without crashing, even if the scatterlist has
size zero, so perhaps we should fix that one as well.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-12 21:26 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 18:44 [PATCH v3 0/7] running kernel mode SIMD with softirqs disabled Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 1/7] crypto: handle zero sized AEAD inputs correctly Ard Biesheuvel
2021-05-12 20:04 ` Eric Biggers
2021-05-12 21:24 ` Ard Biesheuvel [this message]
2021-05-21 7:55 ` Herbert Xu
2021-05-21 9:28 ` Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 2/7] crypto: aead - disallow en/decrypt for non-task or non-softirq context Ard Biesheuvel
2021-05-12 20:06 ` Eric Biggers
2021-05-12 21:24 ` Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 3/7] crypto: skcipher " Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 4/7] crypto: arm64/gcm-aes-ce - remove non-SIMD fallback path Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 5/7] crypto: arm64/aes-neonbs - stop using SIMD helper for skciphers Ard Biesheuvel
2021-05-12 20:08 ` Eric Biggers
2021-05-12 21:25 ` Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 6/7] crypto: arm64/aes-ce " Ard Biesheuvel
2021-05-12 18:44 ` [PATCH v3 7/7] crypto: arm64/aes-ccm - remove non-SIMD fallback path Ard Biesheuvel
2021-05-12 20:11 ` [PATCH v3 0/7] running kernel mode SIMD with softirqs disabled Eric Biggers
2021-05-12 21:31 ` Ard Biesheuvel
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=CAMj1kXHofDrzEs4qc8VNCLpyL-Hc4PSg-JXKTckJvfD6qoK78Q@mail.gmail.com \
--to=ardb@kernel.org \
--cc=ebiggers@kernel.org \
--cc=herbert@gondor.apana.org.au \
--cc=kernel-team@android.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-crypto@vger.kernel.org \
--cc=will@kernel.org \
/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).