From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [net-next PATCH] tls: async support causes out-of-bounds access in crypto APIs Date: Mon, 17 Sep 2018 08:02:07 -0700 (PDT) Message-ID: <20180917.080207.1607312275702268512.davem@davemloft.net> References: <20180914200146.6302.50472.stgit@john-Precision-Tower-5810> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: vakul.garg@nxp.com, davejwatson@fb.com, doronrk@fb.com, netdev@vger.kernel.org, alexei.starovoitov@gmail.com, daniel@iogearbox.net To: john.fastabend@gmail.com Return-path: Received: from shards.monkeyblade.net ([23.128.96.9]:45770 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728554AbeIQU3w (ORCPT ); Mon, 17 Sep 2018 16:29:52 -0400 In-Reply-To: <20180914200146.6302.50472.stgit@john-Precision-Tower-5810> Sender: netdev-owner@vger.kernel.org List-ID: From: John Fastabend 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 Applied, thanks John.