| Message ID | 20180516174841.2119-1-mmullins@fb.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net] tls: don't use stack memory in a scatterlist | expand |
From: Matt Mullins <mmullins@fb.com> Date: Wed, 16 May 2018 10:48:40 -0700 > scatterlist code expects virt_to_page() to work, which fails with > CONFIG_VMAP_STACK=y. > > Fixes: c46234ebb4d1e ("tls: RX path for ktls") > Signed-off-by: Matt Mullins <mmullins@fb.com> Applied and queued up for -stable, thanks. I'm surprised this problem wasn't discovered sooner. How exactly did you discover it? Did you actually see it trigger or is this purely from code inspection?
On Thu, 2018-05-17 at 14:50 -0400, David Miller wrote: > I'm surprised this problem wasn't discovered sooner. How exactly did you > discover it? Did you actually see it trigger or is this purely from code > inspection? Honestly, I'm not sure how it got uncovered, but it was observed at runtime. Doron Roberts-Kedes hit a null pointer dereference so we turned on CONFIG_DEBUG_SG -- then it became a proper BUG_ON(!virt_addr_valid(buf)); in sg_set_buf.
From: Matt Mullins <mmullins@fb.com> Date: Thu, 17 May 2018 20:28:46 +0000 > On Thu, 2018-05-17 at 14:50 -0400, David Miller wrote: >> I'm surprised this problem wasn't discovered sooner. How exactly did you >> discover it? Did you actually see it trigger or is this purely from code >> inspection? > > Honestly, I'm not sure how it got uncovered, but it was observed at > runtime. Doron Roberts-Kedes hit a null pointer dereference so we > turned on CONFIG_DEBUG_SG -- then it became a proper > BUG_ON(!virt_addr_valid(buf)); in sg_set_buf. Fair enough, thanks for the info.
diff --git a/include/net/tls.h b/include/net/tls.h index b400d0bb7448..f5fb16da3860 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -97,6 +97,9 @@ struct tls_sw_context { u8 control; bool decrypted; + char rx_aad_ciphertext[TLS_AAD_SPACE_SIZE]; + char rx_aad_plaintext[TLS_AAD_SPACE_SIZE]; + /* Sending context */ char aad_space[TLS_AAD_SPACE_SIZE]; diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 71e79597f940..e1c93ce74e0f 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -680,7 +680,6 @@ static int decrypt_skb(struct sock *sk, struct sk_buff *skb, struct scatterlist *sgin = &sgin_arr[0]; struct strp_msg *rxm = strp_msg(skb); int ret, nsg = ARRAY_SIZE(sgin_arr); - char aad_recv[TLS_AAD_SPACE_SIZE]; struct sk_buff *unused; ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE, @@ -698,13 +697,13 @@ static int decrypt_skb(struct sock *sk, struct sk_buff *skb, } sg_init_table(sgin, nsg); - sg_set_buf(&sgin[0], aad_recv, sizeof(aad_recv)); + sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE); nsg = skb_to_sgvec(skb, &sgin[1], rxm->offset + tls_ctx->rx.prepend_size, rxm->full_len - tls_ctx->rx.prepend_size); - tls_make_aad(aad_recv, + tls_make_aad(ctx->rx_aad_ciphertext, rxm->full_len - tls_ctx->rx.overhead_size, tls_ctx->rx.rec_seq, tls_ctx->rx.rec_seq_size, @@ -803,12 +802,12 @@ int tls_sw_recvmsg(struct sock *sk, if (to_copy <= len && page_count < MAX_SKB_FRAGS && likely(!(flags & MSG_PEEK))) { struct scatterlist sgin[MAX_SKB_FRAGS + 1]; - char unused[21]; int pages = 0; zc = true; sg_init_table(sgin, MAX_SKB_FRAGS + 1); - sg_set_buf(&sgin[0], unused, 13); + sg_set_buf(&sgin[0], ctx->rx_aad_plaintext, + TLS_AAD_SPACE_SIZE); err = zerocopy_from_iter(sk, &msg->msg_iter, to_copy, &pages,
scatterlist code expects virt_to_page() to work, which fails with CONFIG_VMAP_STACK=y. Fixes: c46234ebb4d1e ("tls: RX path for ktls") Signed-off-by: Matt Mullins <mmullins@fb.com> --- include/net/tls.h | 3 +++ net/tls/tls_sw.c | 9 ++++----- 2 files changed, 7 insertions(+), 5 deletions(-)