Re: crypto: algif_aead - Switch to new AEAD interface

[Stephan Mueller <smueller@chronox.de>]

Am Freitag, 29. Mai 2015, 21:28:40 schrieb Herbert Xu:

Hi Herbert,

>On Fri, May 29, 2015 at 12:09:35PM +0200, Stephan Mueller wrote:
>> > -	return (ctx->used >= (ctx->aead_assoclen + (ctx->enc ? 0 : as)));
>> > +	return ctx->used >= ctx->aead_assoclen + as;
>> 
>> This change requires that the buffer handed in by user space always has
>> room
>> for the tag, regardless whether it is needed or not. Is that intended?
>
>Yes for two reasons.  One is that sometimes we need to enforce
>in-place processing, in which case dst must be at least as big
>as src.  The other reason is to eventually allow in-place processing
>through algif_aead.  Unless the two SG lists were of the same length,
>it isn't possible to do that.

Do we really need to copy in and copy out unneeded data? That sounds very 
inefficient. Besides, can't we leave it to user space to build the right 
memory structure? I.e. if user space wants in-place operation, it needs to 
ensure that the one buffer is sufficient for the requested operation (i.e. 
that the requirements for src lengths and dst lengths are covered).

>> However, when use those changes and I perform the test of
>> libkcapi/test/kcapi -y -s, I get the following strange crash which i have
>> no idea where to look for the cause (normal sendmsg and vmsplice tests
>> with libkcapi/test/kcapi -y and libkcapi/test/kcapi -y -v work flawless)
>
>This is clearly not good.  It looks like memory corruption.

It seems it is triggered by my change suggestions. It is triggered in the 
while() loop in the recvmsg when allocating an output buffer larger than 16 
pages. So, this is nothing in the current upstream code.


Ciao
Stephan