linux-kernel - Re: [PATCH 1/1] crypto: testmgr - call shash

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Fri, 12 Apr 2019 06:34:42 +0000
From:   Lionel DEBIEVE <lionel.debieve@...com>
To:     Eric Biggers <ebiggers@...nel.org>
CC:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        "Alexandre TORGUE" <alexandre.torgue@...com>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Benjamin GAIGNARD <benjamin.gaignard@...com>,
        Fabien DESSENNE <fabien.dessenne@...com>,
        Ludovic BARRE <ludovic.barre@...com>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>
Subject: Re: [PATCH 1/1] crypto: testmgr - call shash_init in crc32c algo

Hi Eric,

On 4/1/19 7:30 PM, Eric Biggers wrote:

> Hi Lionel,
>
> On Mon, Apr 01, 2019 at 02:54:24PM +0200, Lionel Debieve wrote:
>> In case of device call required in low level driver,
>> the context must be initialized before calling the final
>> function.
>>
>> Signed-off-by: Lionel Debieve<lionel.debieve@...com>
>> ---
>>   crypto/testmgr.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
>> index 8386038..4a00d7c 100644
>> --- a/crypto/testmgr.c
>> +++ b/crypto/testmgr.c
>> @@ -2181,6 +2181,13 @@ static int alg_test_crc32c(const struct alg_test_desc *desc,
>>   		shash->tfm = tfm;
>>   		shash->flags = 0;
>>   
>> +		err = crypto_shash_init(shash);
>> +		if (err) {
>> +			printk(KERN_ERR "alg: crc32c: init failed for "
>> +			       "%s: %d\n", driver, err);
>> +			break;
>> +		}
>> +
>>   		*ctx = 420553207;
>>   		err = crypto_shash_final(shash, (u8 *)&val);
>>   		if (err) {
>> -- 
>> 2.7.4
>>
> This defeats the point of the test, which is that crc32c implementations are
> expected to use the same shash_desc context format and be usable by calling
> crypto_shash_update() directly after initializing the context manually, without
> a prior crypto_shash_init().  See for example ext4_chksum() in fs/ext4/ext4.h:
>
> 	static inline u32 ext4_chksum(struct ext4_sb_info *sbi, u32 crc,
> 				      const void *address, unsigned int length)
> 	{
> 		struct {
> 			struct shash_desc shash;
> 			char ctx[4];
> 		} desc;
>
> 		BUG_ON(crypto_shash_descsize(sbi->s_chksum_driver)!=sizeof(desc.ctx));
>
> 		desc.shash.tfm = sbi->s_chksum_driver;
> 		desc.shash.flags = 0;
> 		*(u32 *)desc.ctx = crc;
>
> 		BUG_ON(crypto_shash_update(&desc.shash, address, length));
>
> 		return *(u32 *)desc.ctx;
> 	}
>
> I think you need to fix the stm32 crc32 driver to not store anything extra in
> the shash_desc context, and only use hardware during ->update().
>
> - Eric
>
OK, catch your point but refering to the devel-algos.rst documentation, it seems that there is no way
to bypass the init part. I'm based on a hardware that need a clear initialization to be up and running.
Is the doc not updated? I'm working to optimized data transfer into my hardware by pushing only 32 bits
datas and I'm saving the others to be pushed later.

If we assume that hardware must be only used in the update function, I've to rework the driver as today,
HW init is made in the init function too.

BR
- Lionel