Re: [PATCH 2/11] crypto: scatterwalk - Add missing sg_init_table to scatterwalk_ffwd

[Stephan Mueller <smueller@chronox.de>]

Am Mittwoch, 27. Mai 2015, 17:08:55 schrieb Herbert Xu:

Hi Herbert,

>On Wed, May 27, 2015 at 11:00:55AM +0200, Stephan Mueller wrote:
>> Shouldn't there be a check for src == NULL here? I see the scatterwalk_ffwd
>> being used in the IV generators where they simply use the AD len and
>> others.
>> For AF_ALG, those values may be set by user space in a deliberately wrong
>> way (e.g. more AD len than provided buffers).
>
>algif_aead should be verifying the user provided input.  AFAICS it
>is doing exactly that.  The crash we had previously were due to
>bugs in my algif_aead patch.

To be precise, the concern I currently have are as follows. But I will test it 
later and report back:

The seqiv.c uses the following call:

                        scatterwalk_ffwd(dstbuf, req->dst,
                                         req->assoclen + ivsize),
                        scatterwalk_ffwd(srcbuf, req->src,
                                         req->assoclen + ivsize),

That together with my other tests for seqniv(rfc4106()) this indicates that 
the input SGL must contain AD || IV || PT.

The algif_aead, however only slurps in AD || PT via the sendmsg call and 
processes that as documented in the recvmsg call. So, the IV part is missing 
in the picture as the IV is set via the setsockopt.

So, the aforementioned call unconditionally advances the SGL by AD + 8 bytes 
where I am not sure that the 8 bytes are always accounted for by algif_aead.


Ciao
Stephan