Re: [net-next PATCH] tls: async support causes out-of-bounds access in crypto APIs

[David Miller <davem@davemloft.net>]

From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 14 Sep 2018 13:01:46 -0700

> When async support was added it needed to access the sk from the async
> callback to report errors up the stack. The patch tried to use space
> after the aead request struct by directly setting the reqsize field in
> aead_request. This is an internal field that should not be used
> outside the crypto APIs. It is used by the crypto code to define extra
> space for private structures used in the crypto context. Users of the
> API then use crypto_aead_reqsize() and add the returned amount of
> bytes to the end of the request memory allocation before posting the
> request to encrypt/decrypt APIs.
> 
> So this breaks (with general protection fault and KASAN error, if
> enabled) because the request sent to decrypt is shorter than required
> causing the crypto API out-of-bounds errors. Also it seems unlikely the
> sk is even valid by the time it gets to the callback because of memset
> in crypto layer.
> 
> Anyways, fix this by holding the sk in the skb->sk field when the
> callback is set up and because the skb is already passed through to
> the callback handler via void* we can access it in the handler. Then
> in the handler we need to be careful to NULL the pointer again before
> kfree_skb. I added comments on both the setup (in tls_do_decryption)
> and when we clear it from the crypto callback handler
> tls_decrypt_done(). After this selftests pass again and fixes KASAN
> errors/warnings.
> 
> Fixes: 94524d8fc965 ("net/tls: Add support for async decryption of tls records")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>

Applied, thanks John.