linux-kernel - Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Mar 2018 10:29:45 +0000
From:   Horia Geantă <horia.geanta@....com>
To:     Kamil Konieczny <k.konieczny@...tner.samsung.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Jonathan Corbet <corbet@....net>
CC:     "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: [PATCH v2] crypto: doc - clarify hash callbacks state machine

On 3/20/2018 10:50 AM, Kamil Konieczny wrote:
> On 20.03.2018 08:56, Horia Geantă wrote:
>> Add a note that it is perfectly legal to "abandon" a request object:
>> - call .init() and then (as many times) .update()
>> - _not_ call any of .final(), .finup() or .export() at any point in
>>   future
>>
>> Link: https://lkml.kernel.org/r/20180222114741.ga27...@gondor.apana.org.au
>> Signed-off-by: Horia Geantă <horia.geanta@....com>
>> ---
>>  Documentation/crypto/devel-algos.rst | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/crypto/devel-algos.rst b/Documentation/crypto/devel-algos.rst
>> index 66f50d32dcec..c45c6f400dbd 100644
>> --- a/Documentation/crypto/devel-algos.rst
>> +++ b/Documentation/crypto/devel-algos.rst
>> @@ -236,6 +236,14 @@ when used from another part of the kernel.
>>                                 |
>>                                 '---------------> HASH2
>>  
>> +Note that it is perfectly legal to "abandon" a request object:
>> +- call .init() and then (as many times) .update()
>> +- _not_ call any of .final(), .finup() or .export() at any point in future
>> +
>> +In other words implementations should mind the resource allocation and clean-up.
>> +No resources related to request objects should remain allocated after a call
> -- ^^^^^^^^^^^^
>> +to .init() or .update(), since there might be no chance to free them.
> 
> is it for crypto api  users or for drivers ?
> 
For drivers / providers (below crypto API).

> the creator of request context is responsible for alloc and destroy,
> so why there are no chance of free ?
> 
Hash request object (including request context) is allocated by the user /
client by means of ahash_request_alloc(), and later on freed using
ahash_request_free().
I don't see a problem with this.

However, besides the memory allocated for the request context, other resources
(related to the request) might be needed by the driver.
I provided an example of needing to DMA map a buffer (to load/store HW state
from/to crypto engine), and I am not happy with either solutions:
-DMA map & unmap after each .update()
-Herbert's proposal to use a convoluted DMA mapping scheme

Another example: dynamic memory allocation might be needed beyond what's
available in request context, i.e. driver might not have apriori all the
information needed to inform the tfm about required memory using
crypto_ahash_set_reqsize().

This happens due to the semantics of the crypto API, which allows the user to
initialize a request object and drop it without getting a result (final or
partial hash).
I don't see what below use case is good for, maybe just for benchmarking:
req = ahash_request_alloc();
[...]
crypto_ahash_init(req);
crypto_ahash_update(req);
ahash_request_free(req);

Horia