> -----Original Message-----
> From: John Fastabend <john.fastab...@gmail.com>
> Sent: Saturday, September 15, 2018 1:10 AM
> To: Vakul Garg <vakul.g...@nxp.com>; netdev@vger.kernel.org
> Cc: bor...@mellanox.com; avia...@mellanox.com; davejwat...@fb.com;
> da...@davemloft.net
> Subject: Re: [PATCH net-next v2] net/tls: Add support for async decryption of
> tls records
>
> On 08/29/2018 02:56 AM, Vakul Garg wrote:
> > When tls records are decrypted using asynchronous acclerators such as
> > NXP CAAM engine, the crypto apis return -EINPROGRESS. Presently, on
> > getting -EINPROGRESS, the tls record processing stops till the time
> > the crypto accelerator finishes off and returns the result. This
> > incurs a context switch and is not an efficient way of accessing the
> > crypto accelerators. Crypto accelerators work efficient when they are
> > queued with multiple crypto jobs without having to wait for the
> > previous ones to complete.
> >
> > The patch submits multiple crypto requests without having to wait for
> > for previous ones to complete. This has been implemented for records
> > which are decrypted in zero-copy mode. At the end of recvmsg(), we
> > wait for all the asynchronous decryption requests to complete.
> >
> > The references to records which have been sent for async decryption
> > are dropped. For cases where record decryption is not possible in
> > zero-copy mode, asynchronous decryption is not used and we wait for
> > decryption crypto api to complete.
> >
> > For crypto requests executing in async fashion, the memory for
> > aead_request, sglists and skb etc is freed from the decryption
> > completion handler. The decryption completion handler wakesup the
> > sleeping user context when recvmsg() flags that it has done sending
> > all the decryption requests and there are no more decryption requests
> > pending to be completed.
> >
> > Signed-off-by: Vakul Garg <vakul.g...@nxp.com>
> > Reviewed-by: Dave Watson <davejwat...@fb.com>
> > ---
>
> [...]
>
>
> > @@ -1271,6 +1377,8 @@ int tls_set_sw_offload(struct sock *sk, struct
> tls_context *ctx, int tx)
> > goto free_aead;
> >
> > if (sw_ctx_rx) {
> > + (*aead)->reqsize = sizeof(struct decrypt_req_ctx);
> > +
>
> This is not valid and may cause GPF or best case only a KASAN warning.
> 'reqsize' should probably not be mangled outside the internal crypto APIs but
> the real reason is the reqsize is used to determine how much space is needed
> at the end of the aead_request for crypto private ctx use in encrypt/decrypt.
> After this patch when we submit an aead_request the crypto layer will think it
> has room for its private structs at the end but now only 8B will be there and
> crypto layer will happily memset some arbitrary memory for you amongst
> other things.
>
> Anyways testing a fix now will post shortly.
I am aware that the space in question after aead_request is used internally
by cryptoapi. My plan was to inflate this space by an extra sizeof(struct
decrypt_req_ctx).
(*aead)->reqsize += sizeof(struct decrypt_req_ctx);
Somehow, I missed it.
But your piece of change look a better solution.
Thanks for submitting the fix.
>
> Thanks,
> John
