2021-07-16 16:56:56

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH] crypto: x86/aes-ni - add missing error checks in XTS code

The updated XTS code fails to check the return code of skcipher_walk_virt,
which may lead to skcipher_walk_abort() or skcipher_walk_done() being called
while the walk argument is in an inconsistent state.

So check the return value after each such call, and bail on errors.

Fixes: 2481104fe98d ("crypto: x86/aes-ni-xts - rewrite and drop indirections via glue helper")
Reported-by: Dave Hansen <[email protected]>
Reported-by: syzbot <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/crypto/aesni-intel_glue.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 2144e54a6c89..388643ca2177 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -849,6 +849,8 @@ static int xts_crypt(struct skcipher_request *req, bool encrypt)
return -EINVAL;

err = skcipher_walk_virt(&walk, req, false);
+ if (err)
+ return err;

if (unlikely(tail > 0 && walk.nbytes < walk.total)) {
int blocks = DIV_ROUND_UP(req->cryptlen, AES_BLOCK_SIZE) - 2;
@@ -862,7 +864,10 @@ static int xts_crypt(struct skcipher_request *req, bool encrypt)
skcipher_request_set_crypt(&subreq, req->src, req->dst,
blocks * AES_BLOCK_SIZE, req->iv);
req = &subreq;
+
err = skcipher_walk_virt(&walk, req, false);
+ if (err)
+ return err;
} else {
tail = 0;
}
--
2.30.2


2021-07-16 23:58:59

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH] crypto: x86/aes-ni - add missing error checks in XTS code

On Fri, Jul 16, 2021 at 06:54:03PM +0200, Ard Biesheuvel wrote:
> The updated XTS code fails to check the return code of skcipher_walk_virt,
> which may lead to skcipher_walk_abort() or skcipher_walk_done() being called
> while the walk argument is in an inconsistent state.
>
> So check the return value after each such call, and bail on errors.
>
> Fixes: 2481104fe98d ("crypto: x86/aes-ni-xts - rewrite and drop indirections via glue helper")

Add Cc stable?

> Reported-by: Dave Hansen <[email protected]>
> Reported-by: syzbot <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> arch/x86/crypto/aesni-intel_glue.c | 5 +++++
> 1 file changed, 5 insertions(+)

Reviewed-by: Eric Biggers <[email protected]>

- Eric

2021-07-19 07:12:43

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH] crypto: x86/aes-ni - add missing error checks in XTS code

On Sat, 17 Jul 2021 at 01:58, Eric Biggers <[email protected]> wrote:
>
> On Fri, Jul 16, 2021 at 06:54:03PM +0200, Ard Biesheuvel wrote:
> > The updated XTS code fails to check the return code of skcipher_walk_virt,
> > which may lead to skcipher_walk_abort() or skcipher_walk_done() being called
> > while the walk argument is in an inconsistent state.
> >
> > So check the return value after each such call, and bail on errors.
> >
> > Fixes: 2481104fe98d ("crypto: x86/aes-ni-xts - rewrite and drop indirections via glue helper")
>
> Add Cc stable?
>
> > Reported-by: Dave Hansen <[email protected]>
> > Reported-by: syzbot <[email protected]>
> > Signed-off-by: Ard Biesheuvel <[email protected]>
> > ---
> > arch/x86/crypto/aesni-intel_glue.c | 5 +++++
> > 1 file changed, 5 insertions(+)
>
> Reviewed-by: Eric Biggers <[email protected]>
>

Thanks Eric. Herbert can add the cc:stable if he decides to, but IIRC,
he prefers relying on the fixes: tag only.

2021-07-23 06:55:07

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH] crypto: x86/aes-ni - add missing error checks in XTS code

On Fri, Jul 16, 2021 at 06:54:03PM +0200, Ard Biesheuvel wrote:
> The updated XTS code fails to check the return code of skcipher_walk_virt,
> which may lead to skcipher_walk_abort() or skcipher_walk_done() being called
> while the walk argument is in an inconsistent state.
>
> So check the return value after each such call, and bail on errors.
>
> Fixes: 2481104fe98d ("crypto: x86/aes-ni-xts - rewrite and drop indirections via glue helper")
> Reported-by: Dave Hansen <[email protected]>
> Reported-by: syzbot <[email protected]>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> arch/x86/crypto/aesni-intel_glue.c | 5 +++++
> 1 file changed, 5 insertions(+)

Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt