On 5/17/2019 12:50 PM, Sascha Hauer wrote:
> On Fri, May 17, 2019 at 11:29:04AM +0200, Sascha Hauer wrote:
>> The CAAM driver used to put its debug messages inside #ifdef DEBUG and
>> then prints the messages at KERN_ERR level. Replace this with proper
>> functions printing at KERN_DEBUG level. The #ifdef DEBUG gets
>> unnecessary when the right functions are used.
>>
>> This replaces:
>>
>> - print_hex_dump(KERN_ERR ...) inside #ifdef DEBUG with
>>   print_hex_dump_debug(...)
>> - dev_err() inside #ifdef DEBUG with dev_dbg()
>> - printk(KERN_ERR ...) inside #ifdef DEBUG with dev_dbg()
>>
>> Some parts of the driver use these functions already, so it is only
>> consequent to use the debug function consistently.
>>
>> @@ -993,20 +978,17 @@ static void skcipher_encrypt_done(struct device 
>> *jrdev, u32 *desc, u32 err,
>>      struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
>>      int ivsize = crypto_skcipher_ivsize(skcipher);
>>  
>> -#ifdef DEBUG
>> -    print_hex_dump(KERN_ERR, "dstiv  @"__stringify(__LINE__)": ",
>> +    print_hex_dump_debug("dstiv  @"__stringify(__LINE__)": ",
>>                     DUMP_PREFIX_ADDRESS, 16, 4, req->iv,
>>                     edesc->src_nents > 1 ? 100 : ivsize, 1);
>> -#endif
>> +
> 
> I just realized that this print_hex_dump_debug() needs to be inside if 
> (ivsize)
> because since eaed71a44ad9 ("crypto: caam - add ecb(*) support") req->iv
> can be NULL. This is broken with or without this patch, I can include a
> patch fixing this up when doing a v2.
> 
That's true.
Since this patch set is orthogonal to the bug, IMO the fix shouldn't be part of 
it.

Thanks,
Horia

Reply via email to