linux-kernel - Re: [PATCH 16/18] crypto: talitos - do hw_context DMA mapping outside the requests

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:   Sun, 18 Feb 2018 17:14:03 +0000
From:   Horia Geant <horia.geanta@....com>
To:     Christophe LEROY <christophe.leroy@....fr>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
CC:     "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH 16/18] crypto: talitos - do hw_context DMA mapping outside
 the requests

On 2/17/2018 6:32 PM, Christophe LEROY wrote:
> 
> 
> Le 07/02/2018 à 15:39, Horia Geantă a écrit :
>> On 10/6/2017 4:06 PM, Christophe Leroy wrote:
>>> At every request, we map and unmap the same hash hw_context.
>>>
>>> This patch moves the dma mapping/unmapping in functions ahash_init()
>>> and ahash_import().
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@....fr>
>>> ---
>>>   drivers/crypto/talitos.c | 80 ++++++++++++++++++++++++++++++++++--------------
>>>   1 file changed, 57 insertions(+), 23 deletions(-)
>>>
>>> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
>>> index ebfd6d982ed6..d495649d5267 100644
>>> --- a/drivers/crypto/talitos.c
>>> +++ b/drivers/crypto/talitos.c
>>> @@ -819,6 +819,7 @@ struct talitos_ctx {
>>>   	unsigned int keylen;
>>>   	unsigned int enckeylen;
>>>   	unsigned int authkeylen;
>>> +	dma_addr_t dma_hw_context;
>> This doesn't look correct.
>>
>> talitos_ctx structure is the tfm context.
>> dma_hw_context is the IOVA of hw_context, located in talitos_ahash_req_ctx
>> structure (request context).
> 
> Yes but I have now found how I can know that the request context is 
> being released in order to unmap() dma at that time.
> It is tricky to use the tmf context I agree, but at least I know when 
> tmf context get destroyed, ie in talitos_cra_exit_ahash()
> The request context is created by ahash_request_alloc() and released by
> ahash_request_free(). I have not found the way to call dma_unmap() 
> before ahash_request_free() gets called.
> 
>>
>> If there are multiple requests in flight for the same tfm, dma_hw_context will
>> be overwritten.
> 
> Before overwritting dma_hw_context, it is always released, see 
> talitos_cra_exit_ahash(), ahash_init(), ahash_import()
> 
The problem is not the unmapping.
If there are two requests for the same tfm, then given the following sequence
1. tfm->ahash_init(req1)
	tfm_ctx->dma_hw_context points to req1_ctx->hw_context
2. tfm->ahash_init(req2)
	tfm_ctx->dma_hw_context [unmapped, then] points to req2_ctx->hw_context
i.e. req1 will use the hw_context of req2.

>>
>> dma_hw_context needs to be moved in request context (talitos_ahash_req_ctx struct).
> 
> Any suggestion then on how to handle the issue explained above ?
> 
There is no ahash_exit() callback mirroring ahash_init().

The clean-up of request ctx should be done in the last states of the hash flows
described here:
https://www.kernel.org/doc/html/latest/crypto/devel-algos.html#cipher-definition-with-struct-shash-alg-and-ahash-alg
for e.g. in the final() callback.

Hope this helps,
Horia