Re: [PATCH] crypto: af_alg - add async support to algif_aead

On Mon, Jan 18, 2016 at 07:22:55AM -0800, Tadeusz Struk wrote:

My understanding is that the sock_kmalloc is mainly used for allocations
of the user provided  data, because it keeps tracks of how much memory
is allocated by a socket, and makes sure that is will not exceed the
sysctl_optmem_max limit. Usually the internal structures, with fixed
size are allocated simply with kmalloc. I don't think that using
sock_kmalloc will give us any benefit here.

If there is only ever one of them per-socket then kmalloc is fine,
otherwise you should use sock_kmalloc.

quoted

The code in the aead_recvmsg_sync and _async is very very similar with the 
exception of the areq handling.

What I am wondering, is it possible to consolidate both into one, considering 
that the real difference according to my understanding is the 
af_alg_wait_for_completion usage (in _sync) vs. the use of a self-written 
callback (_async)?

I agree that they are very similar, but I found it much easier to debug
when they are separate functions. I would prefer to keep them separate.
They are also separate in algif_skcipher. It makes it also easier to
read and understand.

I too would prefer a common function.  However we can do this
later if we wish.
 

The inflight ctr is incremented only if an asynchronous request has been
successfully en-queued for processing. If a user forges to call recvmsg
then the function that increments it won't be even called.

quoted

From the other hand we don't want to give the option to interrupt the

wait, because in a case, when we do have request being processed by some
hardware, and the user kills the process, causing the socket to be
freed, then we will get an Oops in the callback.

This should be replaced with a sock_hold.

Cheers,
-- 
Email: Herbert Xu [off-list ref]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt