From:   Daniel Axtens <dja@axtens.net>
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     omosnacek@gmail.com, linux-crypto@vger.kernel.org,
        Herbert Xu <herbert@gondor.apana.org.au>,
        marcelo.cerri@canonical.com, Stephan Mueller <smueller@chronox.de>,
        leo.barbosa@canonical.com, linuxppc-dev@lists.ozlabs.org,
        nayna@linux.ibm.com, pfsmorigo@gmail.com, leitao@debian.org
Subject: Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
In-Reply-To: <20190315022414.GA1671@sol.localdomain>
References: <20190315020901.16509-1-dja@axtens.net> <20190315022414.GA1671@sol.localdomain>
Date:   Fri, 15 Mar 2019 15:24:35 +1100
Message-ID: <875zsku5mk.fsf@dja-thinkpad.axtens.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

Hi Eric,

>> The original assembly imported from OpenSSL has two copy-paste
>> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
>> the code branches to the CBC decryption exit path, rather than to
>> the CTR exit path.
>
> So does this need to be fixed in OpenSSL too?

Yes, I'm getting in touch with some people internally (at IBM) about
doing that.

>> This leads to corruption of the IV, which leads to subsequent blocks
>> being corrupted.
>>=20
>> This can be detected with libkcapi test suite, which is available at
>> https://github.com/smuellerDD/libkcapi
>>=20
>
> Is this also detected by the kernel's crypto self-tests, and if not why n=
ot?
> What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=3Dy?

It seems the self-tests do not catch it. To catch it, there has to be a
test where the blkcipher_walk creates a walk.nbytes such that
[(the number of AES blocks) mod 8] is either 2 or 3. This happens with
AF_ALG pretty frequently, but when I booted with self-tests it only hit
1, 4, 5, 6 and 7 - it missed 0, 2 and 3.

I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
-next?

Regards,
Daniel

>> Reported-by: Ondrej Mosn=C3=A1=C4=8Dek <omosnacek@gmail.com>
>> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by =
ASM")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> ---
>>  drivers/crypto/vmx/aesp8-ppc.pl | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>=20
>> diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-=
ppc.pl
>> index d6a9f63d65ba..de78282b8f44 100644
>> --- a/drivers/crypto/vmx/aesp8-ppc.pl
>> +++ b/drivers/crypto/vmx/aesp8-ppc.pl
>> @@ -1854,7 +1854,7 @@ Lctr32_enc8x_three:
>>  	stvx_u		$out1,$x10,$out
>>  	stvx_u		$out2,$x20,$out
>>  	addi		$out,$out,0x30
>> -	b		Lcbc_dec8x_done
>> +	b		Lctr32_enc8x_done
>>=20=20
>>  .align	5
>>  Lctr32_enc8x_two:
>> @@ -1866,7 +1866,7 @@ Lctr32_enc8x_two:
>>  	stvx_u		$out0,$x00,$out
>>  	stvx_u		$out1,$x10,$out
>>  	addi		$out,$out,0x20
>> -	b		Lcbc_dec8x_done
>> +	b		Lctr32_enc8x_done
>>=20=20
>>  .align	5
>>  Lctr32_enc8x_one:
>> --=20
>> 2.19.1
>>=20