Skip to content

Commit 0aeb54a

Browse files
kuba-mooPaolo Abeni
authored andcommitted
tls: make sure to abort the stream if headers are bogus
Normally we wait for the socket to buffer up the whole record before we service it. If the socket has a tiny buffer, however, we read out the data sooner, to prevent connection stalls. Make sure that we abort the connection when we find out late that the record is actually invalid. Retrying the parsing is fine in itself but since we copy some more data each time before we parse we can overflow the allocated skb space. Constructing a scenario in which we're under pressure without enough data in the socket to parse the length upfront is quite hard. syzbot figured out a way to do this by serving us the header in small OOB sends, and then filling in the recvbuf with a large normal send. Make sure that tls_rx_msg_size() aborts strp, if we reach an invalid record there's really no way to recover. Reported-by: Lee Jones <[email protected]> Fixes: 84c61fe ("tls: rx: do not use the standard strparser") Reviewed-by: Sabrina Dubroca <[email protected]> Signed-off-by: Jakub Kicinski <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 0984710 commit 0aeb54a

File tree

3 files changed

+11
-7
lines changed

3 files changed

+11
-7
lines changed

net/tls/tls.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ void update_sk_prot(struct sock *sk, struct tls_context *ctx);
141141

142142
int wait_on_pending_writer(struct sock *sk, long *timeo);
143143
void tls_err_abort(struct sock *sk, int err);
144+
void tls_strp_abort_strp(struct tls_strparser *strp, int err);
144145

145146
int init_prot_info(struct tls_prot_info *prot,
146147
const struct tls_crypto_info *crypto_info,

net/tls/tls_strp.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
static struct workqueue_struct *tls_strp_wq;
1515

16-
static void tls_strp_abort_strp(struct tls_strparser *strp, int err)
16+
void tls_strp_abort_strp(struct tls_strparser *strp, int err)
1717
{
1818
if (strp->stopped)
1919
return;
@@ -211,11 +211,17 @@ static int tls_strp_copyin_frag(struct tls_strparser *strp, struct sk_buff *skb,
211211
struct sk_buff *in_skb, unsigned int offset,
212212
size_t in_len)
213213
{
214+
unsigned int nfrag = skb->len / PAGE_SIZE;
214215
size_t len, chunk;
215216
skb_frag_t *frag;
216217
int sz;
217218

218-
frag = &skb_shinfo(skb)->frags[skb->len / PAGE_SIZE];
219+
if (unlikely(nfrag >= skb_shinfo(skb)->nr_frags)) {
220+
DEBUG_NET_WARN_ON_ONCE(1);
221+
return -EMSGSIZE;
222+
}
223+
224+
frag = &skb_shinfo(skb)->frags[nfrag];
219225

220226
len = in_len;
221227
/* First make sure we got the header */
@@ -520,10 +526,8 @@ static int tls_strp_read_sock(struct tls_strparser *strp)
520526
tls_strp_load_anchor_with_queue(strp, inq);
521527
if (!strp->stm.full_len) {
522528
sz = tls_rx_msg_size(strp, strp->anchor);
523-
if (sz < 0) {
524-
tls_strp_abort_strp(strp, sz);
529+
if (sz < 0)
525530
return sz;
526-
}
527531

528532
strp->stm.full_len = sz;
529533

net/tls/tls_sw.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2474,8 +2474,7 @@ int tls_rx_msg_size(struct tls_strparser *strp, struct sk_buff *skb)
24742474
return data_len + TLS_HEADER_SIZE;
24752475

24762476
read_failure:
2477-
tls_err_abort(strp->sk, ret);
2478-
2477+
tls_strp_abort_strp(strp, ret);
24792478
return ret;
24802479
}
24812480

0 commit comments

Comments
 (0)