On Wed, 13 Mar 2019 at 06:15, Eric Biggers <ebigg...@kernel.org> wrote:
>
> From: Eric Biggers <ebigg...@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: <sta...@vger.kernel.org> # v5.0+
> Signed-off-by: Eric Biggers <ebigg...@google.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
> 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
>
