In multiple `*_encrypt`, `*_crypt`, `*_decrypt` functions within the x86/crypto
glue files, the `skcipher_walk` structs being used are not properly initialized
prior their usage which can lead to undefined behaviour if the `flags` field of
this structure were to contain junk values at the time of its usage.
This patch series ensures that instances of `struct skcipher_walk` are correctly
initialized across different x86/crypto glue files.
Yuran Pereira (7):
crypto: Fixes uninitialized skcipher_walk use in sm4_aesni_avx_glue
crypto: Fixes uninitialized skcipher_walk use in des3_ede_glue
crypto: Fixes uninitialized skcipher_walk use in chacha_glue
crypto: Fixes uninitialized skcipher_walk use in aesni-intel_glue
crypto: Fixes uninitialized skcipher_walk use in aria_aesni_avx2_glue
crypto: Fixes uninitialized skcipher_walk use in aria_aesni_avx_glue
crypto: Fixes uninitialized skcipher_walk use in aria_gfni_avx512_glue
arch/x86/crypto/aesni-intel_glue.c | 12 ++++++++++++
arch/x86/crypto/aria_aesni_avx2_glue.c | 2 ++
arch/x86/crypto/aria_aesni_avx_glue.c | 2 ++
arch/x86/crypto/aria_gfni_avx512_glue.c | 2 ++
arch/x86/crypto/chacha_glue.c | 2 ++
arch/x86/crypto/des3_ede_glue.c | 4 ++++
arch/x86/crypto/sm4_aesni_avx_glue.c | 7 +++++++
7 files changed, 31 insertions(+)
--
2.25.1
Hey Herbert,
On Thu, Nov 02, 2023 at 12:30:44PM +0800, Herbert Xu wrote:
> On Wed, Nov 01, 2023 at 09:20:43PM -0700, Eric Biggers wrote:
> >
> > Updating all callers of skcipher_walk_virt() seems like the wrong approach.
> > Shouldn't skcipher_walk_virt() be fixed to initialize the flags to 0 instead?
>
> The bits of the flags that are used are initialised in skcipher_walk_next.
>
I noticed that, but since skcipher_walk_first can return with failure, there seems
to be a chance that those bits are never initialized.
> > Also, does this fix affect any behavior, or is it just to fix a KMSAN warning?
> > It needs to be fixed either way, but it's helpful to understand the effect of
> > the fix so that people can decide whether it needs to be backported or not.
>
> Does this actually trigger a KMSAN warning? If so I'd like to see
> it. If it's just a static analyser then I'm not applying this.
No, there is no KMSAN warning. As I mentioned in the individual patches,
they're addressing "coverity" reports (so yes, static analyser).
Initially it did look like a false positive, but upon seeing that
skcipher_walk_first can return without ever calling skcipher_walk_next
I thought that there might be an off-chance that skcipher_walk_virt
returns without ever initializing those bits of the flag... hence this
patch set.
PS: I just saw Eric's reply,
> > Updating all callers of skcipher_walk_virt() seems like the wrong approach.
and realized that maybe my approach is in fact an overkill. Maybe simply initializing
the flags would indeed suffice.
Thanks,
On Thu, Nov 02, 2023 at 10:27:41AM +0530, Yuran Pereira wrote:
>
> I noticed that, but since skcipher_walk_first can return with failure, there seems
> to be a chance that those bits are never initialized.
The API is such that if an error is returned by walk_first/next,
then you must not call into the skcipher_walk_* functions again.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt