From patchwork Wed Jul 22 19:34:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1334139 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BBlyl4jp5z9sRW; Thu, 23 Jul 2020 05:34:22 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jyKVN-0006XR-7K; Wed, 22 Jul 2020 19:34:17 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVL-0006Ws-NJ for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:15 +0000 Received: from 1.general.cascardo.us.vpn ([10.172.70.58] helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVL-0007x6-2b for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:15 +0000 From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU D 1/3] net/tls: fix lowat calculation if some data came from previous record Date: Wed, 22 Jul 2020 16:34:00 -0300 Message-Id: <20200722193402.358366-2-cascardo@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200722193402.358366-1-cascardo@canonical.com> References: <20200722193402.358366-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jakub Kicinski BugLink: https://bugs.launchpad.net/bugs/1888381 If some of the data came from the previous record, i.e. from the rx_list it had already been decrypted, so it's not counted towards the "decrypted" variable, but the "copied" variable. Take that into account when checking lowat. When calculating lowat target we need to pass the original len. E.g. if lowat is at 80, len is 100 and we had 30 bytes on rx_list target would currently be incorrectly calculated as 70, even though we only need 50 more bytes to make up the 80. Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records") Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Tested-by: David Beckett Signed-off-by: David S. Miller (cherry picked from commit 46a1695960d0600d58da7af33c65f24f3d839674) Signed-off-by: Thadeu Lima de Souza Cascardo --- net/tls/tls_sw.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index e903b9f85108..b800e8b500e8 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1603,13 +1603,12 @@ int tls_sw_recvmsg(struct sock *sk, copied = err; } - len = len - copied; - if (len) { - target = sock_rcvlowat(sk, flags & MSG_WAITALL, len); - timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT); - } else { + if (len <= copied) goto recv_end; - } + + target = sock_rcvlowat(sk, flags & MSG_WAITALL, len); + len = len - copied; + timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT); do { bool retain_skb = false; @@ -1715,7 +1714,7 @@ int tls_sw_recvmsg(struct sock *sk, } /* If we have a new message from strparser, continue now. */ - if (decrypted >= target && !ctx->recv_pkt) + if (decrypted + copied >= target && !ctx->recv_pkt) break; } while (len); From patchwork Wed Jul 22 19:34:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1334142 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BBlyn2mX7z9sTK; Thu, 23 Jul 2020 05:34:25 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jyKVR-0006YO-Bk; Wed, 22 Jul 2020 19:34:21 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVM-0006XG-Sd for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:16 +0000 Received: from 1.general.cascardo.us.vpn ([10.172.70.58] helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVM-0007x6-7B for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:16 +0000 From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU D 2/3] net/tls: fix no wakeup on partial reads Date: Wed, 22 Jul 2020 16:34:01 -0300 Message-Id: <20200722193402.358366-3-cascardo@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200722193402.358366-1-cascardo@canonical.com> References: <20200722193402.358366-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jakub Kicinski BugLink: https://bugs.launchpad.net/bugs/1888381 When tls_sw_recvmsg() partially copies a record it pops that record from ctx->recv_pkt and places it on rx_list. Next iteration of tls_sw_recvmsg() reads from rx_list via process_rx_list() before it enters the decryption loop. If there is no more records to be read tls_wait_data() will put the process on the wait queue and got to sleep. This is incorrect, because some data was already copied in process_rx_list(). In case of RPC connections process may never get woken up, because peer also simply blocks in read(). I think this may also fix a similar issue when BPF is at play, because after __tcp_bpf_recvmsg() returns some data we subtract it from len and use continue to restart the loop, but len could have just reached 0, so again we'd sleep unnecessarily. That's added by: commit d3b18ad31f93 ("tls: add bpf support to sk_msg handling") Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records") Reported-by: David Beckett Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Tested-by: David Beckett Signed-off-by: David S. Miller (cherry picked from commit 04b25a5411f966c2e586909a8496553b71876fae) Signed-off-by: Thadeu Lima de Souza Cascardo --- net/tls/tls_sw.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index b800e8b500e8..74513bcb3824 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1610,7 +1610,7 @@ int tls_sw_recvmsg(struct sock *sk, len = len - copied; timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT); - do { + while (len && (decrypted + copied < target || ctx->recv_pkt)) { bool retain_skb = false; bool async = false; bool zc = false; @@ -1712,11 +1712,7 @@ int tls_sw_recvmsg(struct sock *sk, } else { break; } - - /* If we have a new message from strparser, continue now. */ - if (decrypted + copied >= target && !ctx->recv_pkt) - break; - } while (len); + } recv_end: if (num_async) { From patchwork Wed Jul 22 19:34:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1334141 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4BBlyn1qRdz9sT6; Thu, 23 Jul 2020 05:34:25 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jyKVR-0006Yh-Ff; Wed, 22 Jul 2020 19:34:21 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVN-0006Xp-Vi for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:17 +0000 Received: from 1.general.cascardo.us.vpn ([10.172.70.58] helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jyKVN-0007x6-C5 for kernel-team@lists.ubuntu.com; Wed, 22 Jul 2020 19:34:17 +0000 From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU D 3/3] net/tls: fix poll ignoring partially copied records Date: Wed, 22 Jul 2020 16:34:02 -0300 Message-Id: <20200722193402.358366-4-cascardo@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200722193402.358366-1-cascardo@canonical.com> References: <20200722193402.358366-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jakub Kicinski BugLink: https://bugs.launchpad.net/bugs/1888381 David reports that RPC applications which use epoll() occasionally get stuck, and that TLS ULP causes the kernel to not wake applications, even though read() will return data. This is indeed true. The ctx->rx_list which holds partially copied records is not consulted when deciding whether socket is readable. Note that SO_RCVLOWAT with epoll() is and has always been broken for kernel TLS. We'd need to parse all records from the TCP layer, instead of just the first one. Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple records") Reported-by: David Beckett Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller (cherry picked from commit 13aecb17acabc2a92187d08f7ca93bb8aad62c6f) Signed-off-by: Thadeu Lima de Souza Cascardo --- net/tls/tls_sw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 74513bcb3824..72e21915ddfa 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1830,7 +1830,8 @@ bool tls_sw_stream_read(const struct sock *sk) ingress_empty = list_empty(&psock->ingress_msg); rcu_read_unlock(); - return !ingress_empty || ctx->recv_pkt; + return !ingress_empty || ctx->recv_pkt || + !skb_queue_empty(&ctx->rx_list); } static int tls_read_size(struct strparser *strp, struct sk_buff *skb)