From 46a1695960d0600d58da7af33c65f24f3d839674 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 24 May 2019 10:34:30 -0700 Subject: net/tls: fix lowat calculation if some data came from previous record 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 --- net/tls/tls_sw.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index d93f83f77864..fc13234db74a 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1712,13 +1712,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; @@ -1853,7 +1852,7 @@ pick_next_record: } /* 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); -- cgit From 04b25a5411f966c2e586909a8496553b71876fae Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 24 May 2019 10:34:32 -0700 Subject: net/tls: fix no wakeup on partial reads 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 --- net/tls/tls_sw.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'net/tls/tls_sw.c') diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index fc13234db74a..960494f437ac 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -1719,7 +1719,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 zc = false; int to_decrypt; @@ -1850,11 +1850,7 @@ pick_next_record: } 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) { -- cgit