[PATCH net-next v9 2/5] tls: Fix dangling skb pointer in tls_sw_read_sock()

[Chuck Lever <cel@kernel.org>]

From: Chuck Lever <chuck.lever@oracle.com>

Two related defects in the receive loop of tls_sw_read_sock()
share a single fix.

Per ISO/IEC 9899:2011 section 6.2.4p2, a pointer value becomes
indeterminate when the object it points to reaches the end of
its lifetime; Annex J.2 classifies the use of such a value as
undefined behavior. consume_skb(skb) in the fully-consumed path
frees the skb, but the "do { } while (skb)" loop condition then
evaluates that freed pointer. Although the value is never
dereferenced -- the loop either continues and overwrites skb,
or exits -- any future change that adds a dereference between
consume_skb() and the loop condition would produce a silent
use-after-free.

Separately, when read_actor() consumes only part of a record
(used < rxm->full_len) but desc->count is still non-zero, the
existing code updates rxm in place and falls through to the
next loop iteration. The next iteration then unconditionally
overwrites skb without freeing or requeuing the partially
consumed buffer, leaking the skb and silently dropping stream
data.

A read_actor returning fewer bytes than offered is in every
case a backpressure signal; the only correct response is to
requeue and exit. Replace the do/while with an explicit
for(;;), requeue unconditionally on partial consume, and break
on exhausted desc->count after a full consume.

Fixes: 662fbcec32f4 ("net/tls: implement ->read_sock()")
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/tls/tls_sw.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 244ac8ed4b01..c58d3b0b0a8a 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2366,7 +2366,7 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		goto read_sock_end;
 
 	decrypted = 0;
-	do {
+	for (;;) {
 		if (!skb_queue_empty(&ctx->rx_list)) {
 			skb = __skb_dequeue(&ctx->rx_list);
 			rxm = strp_msg(skb);
@@ -2411,14 +2411,13 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t *desc,
 		if (used < rxm->full_len) {
 			rxm->offset += used;
 			rxm->full_len -= used;
-			if (!desc->count)
-				goto read_sock_requeue;
-		} else {
-			consume_skb(skb);
-			if (!desc->count)
-				skb = NULL;
+			goto read_sock_requeue;
 		}
-	} while (skb);
+		consume_skb(skb);
+		skb = NULL;
+		if (!desc->count)
+			break;
+	}
 
 read_sock_end:
 	tls_rx_reader_release(sk, ctx);

-- 
2.53.0