| From 08e6ad96836b4736198ee65b47e8859e9b25fca0 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 28 Feb 2024 23:44:00 +0100 |
| Subject: tls: fix use-after-free on failed backlog decryption |
| |
| From: Sabrina Dubroca <sd@queasysnail.net> |
| |
| [ Upstream commit 13114dc5543069f7b97991e3b79937b6da05f5b0 ] |
| |
| When the decrypt request goes to the backlog and crypto_aead_decrypt |
| returns -EBUSY, tls_do_decryption will wait until all async |
| decryptions have completed. If one of them fails, tls_do_decryption |
| will return -EBADMSG and tls_decrypt_sg jumps to the error path, |
| releasing all the pages. But the pages have been passed to the async |
| callback, and have already been released by tls_decrypt_done. |
| |
| The only true async case is when crypto_aead_decrypt returns |
| -EINPROGRESS. With -EBUSY, we already waited so we can tell |
| tls_sw_recvmsg that the data is available for immediate copy, but we |
| need to notify tls_decrypt_sg (via the new ->async_done flag) that the |
| memory has already been released. |
| |
| Fixes: 859054147318 ("net: tls: handle backlogging of crypto requests") |
| Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> |
| Link: https://lore.kernel.org/r/4755dd8d9bebdefaa19ce1439b833d6199d4364c.1709132643.git.sd@queasysnail.net |
| Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/tls/tls_sw.c | 24 +++++++++++++++++------- |
| 1 file changed, 17 insertions(+), 7 deletions(-) |
| |
| diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c |
| index 733dbae9b057c..acf5bb74fd386 100644 |
| --- a/net/tls/tls_sw.c |
| +++ b/net/tls/tls_sw.c |
| @@ -52,6 +52,7 @@ struct tls_decrypt_arg { |
| struct_group(inargs, |
| bool zc; |
| bool async; |
| + bool async_done; |
| u8 tail; |
| ); |
| |
| @@ -286,15 +287,18 @@ static int tls_do_decryption(struct sock *sk, |
| } |
| |
| ret = crypto_aead_decrypt(aead_req); |
| + if (ret == -EINPROGRESS) |
| + return 0; |
| + |
| if (ret == -EBUSY) { |
| ret = tls_decrypt_async_wait(ctx); |
| - ret = ret ?: -EINPROGRESS; |
| - } |
| - if (ret == -EINPROGRESS) { |
| - return 0; |
| - } else if (darg->async) { |
| - atomic_dec(&ctx->decrypt_pending); |
| + darg->async_done = true; |
| + /* all completions have run, we're not doing async anymore */ |
| + darg->async = false; |
| + return ret; |
| } |
| + |
| + atomic_dec(&ctx->decrypt_pending); |
| darg->async = false; |
| |
| return ret; |
| @@ -1593,8 +1597,11 @@ static int tls_decrypt_sg(struct sock *sk, struct iov_iter *out_iov, |
| /* Prepare and submit AEAD request */ |
| err = tls_do_decryption(sk, sgin, sgout, dctx->iv, |
| data_len + prot->tail_size, aead_req, darg); |
| - if (err) |
| + if (err) { |
| + if (darg->async_done) |
| + goto exit_free_skb; |
| goto exit_free_pages; |
| + } |
| |
| darg->skb = clear_skb ?: tls_strp_msg(ctx); |
| clear_skb = NULL; |
| @@ -1606,6 +1613,9 @@ static int tls_decrypt_sg(struct sock *sk, struct iov_iter *out_iov, |
| return err; |
| } |
| |
| + if (unlikely(darg->async_done)) |
| + return 0; |
| + |
| if (prot->tail_size) |
| darg->tail = dctx->tail; |
| |
| -- |
| 2.43.0 |
| |