From: Eric Biggers <ebiggers@kernel.org>
To: linux-crypto@vger.kernel.org, Herbert Xu <herbert@gondor.apana.org.au>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
linux-arm-kernel@lists.infradead.org, x86@kernel.org
Subject: [PATCH 1/8] crypto: chacha-generic - fix use as arm64 no-NEON fallback
Date: Tue, 12 Mar 2019 22:12:45 -0700 [thread overview]
Message-ID: <20190313051252.2917-2-ebiggers@kernel.org> (raw)
In-Reply-To: <20190313051252.2917-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
The arm64 implementations of ChaCha and XChaCha are failing the extra
crypto self-tests following my patches to test the !may_use_simd() code
paths, which previously were untested. The problem is as follows:
When !may_use_simd(), the arm64 NEON implementations fall back to the
generic implementation, which uses the skcipher_walk API to iterate
through the src/dst scatterlists. Due to how the skcipher_walk API
works, walk.stride is set from the skcipher_alg actually being used,
which in this case is the arm64 NEON algorithm. Thus walk.stride is
5*CHACHA_BLOCK_SIZE, not CHACHA_BLOCK_SIZE.
This unnecessarily large stride shouldn't cause an actual problem.
However, the generic implementation computes round_down(nbytes,
walk.stride). round_down() assumes the round amount is a power of 2,
which 5*CHACHA_BLOCK_SIZE is not, so it gives the wrong result.
This causes the following case in skcipher_walk_done() to be hit,
causing a WARN() and failing the encryption operation:
if (WARN_ON(err)) {
/* unexpected case; didn't process all bytes */
err = -EINVAL;
goto finish;
}
Fix it by rounding down to CHACHA_BLOCK_SIZE instead of walk.stride.
(Or we could replace round_down() with rounddown(), but that would add a
slow division operation every time, which I think we should avoid.)
Fixes: 2fe55987b262 ("crypto: arm64/chacha - use combined SIMD/ALU routine for more speed")
Cc: <stable@vger.kernel.org> # v5.0+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/chacha_generic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/chacha_generic.c b/crypto/chacha_generic.c
index 35b583101f4f..90ec0ec1b4f7 100644
--- a/crypto/chacha_generic.c
+++ b/crypto/chacha_generic.c
@@ -52,7 +52,7 @@ static int chacha_stream_xor(struct skcipher_request *req,
unsigned int nbytes = walk.nbytes;
if (nbytes < walk.total)
- nbytes = round_down(nbytes, walk.stride);
+ nbytes = round_down(nbytes, CHACHA_BLOCK_SIZE);
chacha_docrypt(state, walk.dst.virt.addr, walk.src.virt.addr,
nbytes, ctx->nrounds);
--
2.21.0
WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-crypto@vger.kernel.org, Herbert Xu <herbert@gondor.apana.org.au>
Cc: x86@kernel.org, linux-arm-kernel@lists.infradead.org,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH 1/8] crypto: chacha-generic - fix use as arm64 no-NEON fallback
Date: Tue, 12 Mar 2019 22:12:45 -0700 [thread overview]
Message-ID: <20190313051252.2917-2-ebiggers@kernel.org> (raw)
In-Reply-To: <20190313051252.2917-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
The arm64 implementations of ChaCha and XChaCha are failing the extra
crypto self-tests following my patches to test the !may_use_simd() code
paths, which previously were untested. The problem is as follows:
When !may_use_simd(), the arm64 NEON implementations fall back to the
generic implementation, which uses the skcipher_walk API to iterate
through the src/dst scatterlists. Due to how the skcipher_walk API
works, walk.stride is set from the skcipher_alg actually being used,
which in this case is the arm64 NEON algorithm. Thus walk.stride is
5*CHACHA_BLOCK_SIZE, not CHACHA_BLOCK_SIZE.
This unnecessarily large stride shouldn't cause an actual problem.
However, the generic implementation computes round_down(nbytes,
walk.stride). round_down() assumes the round amount is a power of 2,
which 5*CHACHA_BLOCK_SIZE is not, so it gives the wrong result.
This causes the following case in skcipher_walk_done() to be hit,
causing a WARN() and failing the encryption operation:
if (WARN_ON(err)) {
/* unexpected case; didn't process all bytes */
err = -EINVAL;
goto finish;
}
Fix it by rounding down to CHACHA_BLOCK_SIZE instead of walk.stride.
(Or we could replace round_down() with rounddown(), but that would add a
slow division operation every time, which I think we should avoid.)
Fixes: 2fe55987b262 ("crypto: arm64/chacha - use combined SIMD/ALU routine for more speed")
Cc: <stable@vger.kernel.org> # v5.0+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
crypto/chacha_generic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/chacha_generic.c b/crypto/chacha_generic.c
index 35b583101f4f..90ec0ec1b4f7 100644
--- a/crypto/chacha_generic.c
+++ b/crypto/chacha_generic.c
@@ -52,7 +52,7 @@ static int chacha_stream_xor(struct skcipher_request *req,
unsigned int nbytes = walk.nbytes;
if (nbytes < walk.total)
- nbytes = round_down(nbytes, walk.stride);
+ nbytes = round_down(nbytes, CHACHA_BLOCK_SIZE);
chacha_docrypt(state, walk.dst.virt.addr, walk.src.virt.addr,
nbytes, ctx->nrounds);
--
2.21.0
_______________________________________________
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:[~2019-03-13 5:15 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-13 5:12 [PATCH 0/8] crypto: test the !may_use_simd() fallback code Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 5:12 ` Eric Biggers [this message]
2019-03-13 5:12 ` [PATCH 1/8] crypto: chacha-generic - fix use as arm64 no-NEON fallback Eric Biggers
2019-03-13 7:50 ` Ard Biesheuvel
2019-03-13 7:50 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 2/8] crypto: arm64/gcm-aes-ce - fix no-NEON fallback code Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:29 ` Ard Biesheuvel
2019-03-13 10:29 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 3/8] crypto: simd,testmgr - introduce crypto_simd_usable() Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:31 ` Ard Biesheuvel
2019-03-13 10:31 ` [PATCH 3/8] crypto: simd, testmgr " Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 4/8] crypto: x86 - convert to use crypto_simd_usable() Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:32 ` Ard Biesheuvel
2019-03-13 10:32 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 5/8] crypto: arm " Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:33 ` Ard Biesheuvel
2019-03-13 10:33 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 6/8] crypto: arm64 " Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:33 ` Ard Biesheuvel
2019-03-13 10:33 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 7/8] crypto: simd " Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:34 ` Ard Biesheuvel
2019-03-13 10:34 ` Ard Biesheuvel
2019-03-13 5:12 ` [PATCH 8/8] crypto: testmgr - test the !may_use_simd() fallback code Eric Biggers
2019-03-13 5:12 ` Eric Biggers
2019-03-13 10:35 ` Ard Biesheuvel
2019-03-13 10:35 ` Ard Biesheuvel
2019-03-13 10:50 ` [PATCH 0/8] crypto: " Ard Biesheuvel
2019-03-13 10:50 ` Ard Biesheuvel
2019-03-22 13:03 ` Herbert Xu
2019-03-22 13:03 ` Herbert Xu
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=20190313051252.2917-2-ebiggers@kernel.org \
--to=ebiggers@kernel.org \
--cc=ard.biesheuvel@linaro.org \
--cc=herbert@gondor.apana.org.au \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-crypto@vger.kernel.org \
--cc=x86@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 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.