Skip to content

Commit 6db015f

Browse files
committed
tls: handle data disappearing from under the TLS ULP
TLS expects that it owns the receive queue of the TCP socket. This cannot be guaranteed in case the reader of the TCP socket entered before the TLS ULP was installed, or uses some non-standard read API (eg. zerocopy ones). Replace the WARN_ON() and a buggy early exit (which leaves anchor pointing to a freed skb) with real error handling. Wipe the parsing state and tell the reader to retry. We already reload the anchor every time we (re)acquire the socket lock, so the only condition we need to avoid is an out of bounds read (not having enough bytes in the socket for previously parsed record len). If some data was read from under TLS but there's enough in the queue we'll reload and decrypt what is most likely not a valid TLS record. Leading to some undefined behavior from TLS perspective (corrupting a stream? missing an alert? missing an attack?) but no kernel crash should take place. Reported-by: William Liu <[email protected]> Reported-by: Savino Dicanosa <[email protected]> Link: https://lore.kernel.org/tFjq_kf7sWIG3A7CrCg_egb8CVsT_gsmHAK0_wxDPJXfIzxFAMxqmLwp3MlU5EHiet0AwwJldaaFdgyHpeIUCS-3m3llsmRzp9xIOBR4lAI=@syst3mfailure.io Fixes: 84c61fe ("tls: rx: do not use the standard strparser") Reviewed-by: Eric Dumazet <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 2efe412 commit 6db015f

File tree

3 files changed

+11
-5
lines changed

3 files changed

+11
-5
lines changed

net/tls/tls.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ void tls_strp_msg_done(struct tls_strparser *strp);
196196
int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb);
197197
void tls_rx_msg_ready(struct tls_strparser *strp);
198198

199-
void tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh);
199+
bool tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh);
200200
int tls_strp_msg_cow(struct tls_sw_context_rx *ctx);
201201
struct sk_buff *tls_strp_msg_detach(struct tls_sw_context_rx *ctx);
202202
int tls_strp_msg_hold(struct tls_strparser *strp, struct sk_buff_head *dst);

net/tls/tls_strp.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ static void tls_strp_load_anchor_with_queue(struct tls_strparser *strp, int len)
475475
strp->stm.offset = offset;
476476
}
477477

478-
void tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh)
478+
bool tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh)
479479
{
480480
struct strp_msg *rxm;
481481
struct tls_msg *tlm;
@@ -484,8 +484,11 @@ void tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh)
484484
DEBUG_NET_WARN_ON_ONCE(!strp->stm.full_len);
485485

486486
if (!strp->copy_mode && force_refresh) {
487-
if (WARN_ON(tcp_inq(strp->sk) < strp->stm.full_len))
488-
return;
487+
if (unlikely(tcp_inq(strp->sk) < strp->stm.full_len)) {
488+
WRITE_ONCE(strp->msg_ready, 0);
489+
memset(&strp->stm, 0, sizeof(strp->stm));
490+
return false;
491+
}
489492

490493
tls_strp_load_anchor_with_queue(strp, strp->stm.full_len);
491494
}
@@ -495,6 +498,8 @@ void tls_strp_msg_load(struct tls_strparser *strp, bool force_refresh)
495498
rxm->offset = strp->stm.offset;
496499
tlm = tls_msg(strp->anchor);
497500
tlm->control = strp->mark;
501+
502+
return true;
498503
}
499504

500505
/* Called with lock held on lower socket */

net/tls/tls_sw.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,8 @@ tls_rx_rec_wait(struct sock *sk, struct sk_psock *psock, bool nonblock,
13841384
return sock_intr_errno(timeo);
13851385
}
13861386

1387-
tls_strp_msg_load(&ctx->strp, released);
1387+
if (unlikely(!tls_strp_msg_load(&ctx->strp, released)))
1388+
return tls_rx_rec_wait(sk, psock, nonblock, false);
13881389

13891390
return 1;
13901391
}

0 commit comments

Comments
 (0)