From 00ec9612a619adccf029f51f9c0bd9e1784107a2 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 29 Feb 2024 14:36:42 +0800 Subject: [PATCH 1/4] tls: rx: simplify async wait mainline inclusion from mainline-v5.19-rc1 commit 37943f047bfb88ba4dfc7a522563f57c86d088a0 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I92REK CVE: CVE-2024-26583 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=37943f047bfb88ba4dfc7a522563f57c86d088a0 -------------------------------- Since we are protected from async completions by decrypt_compl_lock we can drop the async_notify and reinit the completion before we start waiting. Signed-off-by: Jakub Kicinski Signed-off-by: David S. Miller Signed-off-by: Ziyang Xuan (cherry picked from commit 300a4e32caaf7fca8c3022ce9ac9f2efff7b3f08) --- include/net/tls.h | 1 - net/tls/tls_sw.c | 14 ++------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index c837ef871564..6654cb041693 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -161,7 +161,6 @@ struct tls_sw_context_rx { atomic_t decrypt_pending; /* protect crypto_wait with decrypt_pending*/ spinlock_t decrypt_compl_lock; - bool async_notify; }; struct tls_record_info { diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 44f7f586f5fd..0e7ea38fe3c3 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -169,7 +169,6 @@ static void tls_decrypt_done(struct crypto_async_request *req, int err) struct scatterlist *sg; struct sk_buff *skb; unsigned int pages; - int pending; skb = (struct sk_buff *)req->data; tls_ctx = tls_get_ctx(skb->sk); @@ -217,9 +216,7 @@ static void tls_decrypt_done(struct crypto_async_request *req, int err) kfree(aead_req); spin_lock_bh(&ctx->decrypt_compl_lock); - pending = atomic_dec_return(&ctx->decrypt_pending); - - if (!pending && ctx->async_notify) + if (!atomic_dec_return(&ctx->decrypt_pending)) complete(&ctx->async_wait.completion); spin_unlock_bh(&ctx->decrypt_compl_lock); } @@ -1950,7 +1947,7 @@ int tls_sw_recvmsg(struct sock *sk, if (num_async) { /* Wait for all previously submitted records to be decrypted */ spin_lock_bh(&ctx->decrypt_compl_lock); - ctx->async_notify = true; + reinit_completion(&ctx->async_wait.completion); pending = atomic_read(&ctx->decrypt_pending); spin_unlock_bh(&ctx->decrypt_compl_lock); if (pending) { @@ -1962,15 +1959,8 @@ int tls_sw_recvmsg(struct sock *sk, decrypted = 0; goto end; } - } else { - reinit_completion(&ctx->async_wait.completion); } - /* There can be no concurrent accesses, since we have no - * pending decrypt operations - */ - WRITE_ONCE(ctx->async_notify, false); - /* Drain records from the rx_list & copy if required */ if (is_peek || is_kvec) err = process_rx_list(ctx, msg, &control, &cmsg, copied, -- Gitee From 2338a68eff4c7c8154a28f4c61168571aa0dba6a Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 29 Feb 2024 14:36:43 +0800 Subject: [PATCH 2/4] net: tls: factor out tls_*crypt_async_wait() mainline inclusion from mainline-v6.8-rc5 commit c57ca512f3b68ddcd62bda9cc24a8f5584ab01b1 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I92REK CVE: CVE-2024-26583 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c57ca512f3b68ddcd62bda9cc24a8f5584ab01b1 -------------------------------- Factor out waiting for async encrypt and decrypt to finish. There are already multiple copies and a subsequent fix will need more. No functional changes. Note that crypto_wait_req() returns wait->err Signed-off-by: Jakub Kicinski Reviewed-by: Simon Horman Reviewed-by: Sabrina Dubroca Signed-off-by: David S. Miller Conflicts: net/tls/tls_sw.c Signed-off-by: Ziyang Xuan (cherry picked from commit 91bb7b8b0c6d6d8b32b653bf3490ab4436561c48) --- net/tls/tls_sw.c | 89 ++++++++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 40 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 0e7ea38fe3c3..718f682fc7ed 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -221,6 +221,20 @@ static void tls_decrypt_done(struct crypto_async_request *req, int err) spin_unlock_bh(&ctx->decrypt_compl_lock); } +static int tls_decrypt_async_wait(struct tls_sw_context_rx *ctx) +{ + int pending; + + spin_lock_bh(&ctx->decrypt_compl_lock); + reinit_completion(&ctx->async_wait.completion); + pending = atomic_read(&ctx->decrypt_pending); + spin_unlock_bh(&ctx->decrypt_compl_lock); + if (pending) + crypto_wait_req(-EINPROGRESS, &ctx->async_wait); + + return ctx->async_wait.err; +} + static int tls_do_decryption(struct sock *sk, struct sk_buff *skb, struct scatterlist *sgin, @@ -491,6 +505,28 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) schedule_delayed_work(&ctx->tx_work.work, 1); } +static int tls_encrypt_async_wait(struct tls_sw_context_tx *ctx) +{ + int pending; + + spin_lock_bh(&ctx->encrypt_compl_lock); + ctx->async_notify = true; + + pending = atomic_read(&ctx->encrypt_pending); + spin_unlock_bh(&ctx->encrypt_compl_lock); + if (pending) + crypto_wait_req(-EINPROGRESS, &ctx->async_wait); + else + reinit_completion(&ctx->async_wait.completion); + + /* There can be no concurrent accesses, since we have no + * pending encrypt operations + */ + WRITE_ONCE(ctx->async_notify, false); + + return ctx->async_wait.err; +} + static int tls_do_encryption(struct sock *sk, struct tls_context *tls_ctx, struct tls_sw_context_tx *ctx, @@ -945,7 +981,6 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) int num_zc = 0; int orig_size; int ret = 0; - int pending; if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_CMSG_COMPAT)) @@ -1112,24 +1147,12 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) if (!num_async) { goto send_end; } else if (num_zc) { - /* Wait for pending encryptions to get completed */ - spin_lock_bh(&ctx->encrypt_compl_lock); - ctx->async_notify = true; - - pending = atomic_read(&ctx->encrypt_pending); - spin_unlock_bh(&ctx->encrypt_compl_lock); - if (pending) - crypto_wait_req(-EINPROGRESS, &ctx->async_wait); - else - reinit_completion(&ctx->async_wait.completion); - - /* There can be no concurrent accesses, since we have no - * pending encrypt operations - */ - WRITE_ONCE(ctx->async_notify, false); + int err; - if (ctx->async_wait.err) { - ret = ctx->async_wait.err; + /* Wait for pending encryptions to get completed */ + err = tls_encrypt_async_wait(ctx); + if (err) { + ret = err; copied = 0; } } @@ -1768,7 +1791,6 @@ int tls_sw_recvmsg(struct sock *sk, bool is_peek = flags & MSG_PEEK; bool bpf_strp_enabled; int num_async = 0; - int pending; flags |= nonblock; @@ -1946,19 +1968,13 @@ int tls_sw_recvmsg(struct sock *sk, recv_end: if (num_async) { /* Wait for all previously submitted records to be decrypted */ - spin_lock_bh(&ctx->decrypt_compl_lock); - reinit_completion(&ctx->async_wait.completion); - pending = atomic_read(&ctx->decrypt_pending); - spin_unlock_bh(&ctx->decrypt_compl_lock); - if (pending) { - err = crypto_wait_req(-EINPROGRESS, &ctx->async_wait); - if (err) { - /* one of async decrypt failed */ - tls_err_abort(sk, err); - copied = 0; - decrypted = 0; - goto end; - } + err = tls_decrypt_async_wait(ctx); + if (err) { + /* one of async decrypt failed */ + tls_err_abort(sk, err); + copied = 0; + decrypted = 0; + goto end; } /* Drain records from the rx_list & copy if required */ @@ -2172,16 +2188,9 @@ void tls_sw_release_resources_tx(struct sock *sk) struct tls_context *tls_ctx = tls_get_ctx(sk); struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx); struct tls_rec *rec, *tmp; - int pending; /* Wait for any pending async encryptions to complete */ - spin_lock_bh(&ctx->encrypt_compl_lock); - ctx->async_notify = true; - pending = atomic_read(&ctx->encrypt_pending); - spin_unlock_bh(&ctx->encrypt_compl_lock); - - if (pending) - crypto_wait_req(-EINPROGRESS, &ctx->async_wait); + tls_encrypt_async_wait(ctx); tls_tx_records(sk, -1); -- Gitee From fb9e1261f42904f916d30c5d5f45d91e4fee6291 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 29 Feb 2024 14:36:44 +0800 Subject: [PATCH 3/4] tls: fix race between async notify and socket close mainline inclusion from mainline-v6.8-rc5 commit aec7961916f3f9e88766e2688992da6980f11b8d category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I92REK CVE: CVE-2024-26583 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=aec7961916f3f9e88766e2688992da6980f11b8d -------------------------------- The submitting thread (one which called recvmsg/sendmsg) may exit as soon as the async crypto handler calls complete() so any code past that point risks touching already freed data. Try to avoid the locking and extra flags altogether. Have the main thread hold an extra reference, this way we can depend solely on the atomic ref counter for synchronization. Don't futz with reiniting the completion, either, we are now tightly controlling when completion fires. Reported-by: valis Fixes: 0cada33241d9 ("net/tls: fix race condition causing kernel panic") Signed-off-by: Jakub Kicinski Reviewed-by: Simon Horman Reviewed-by: Eric Dumazet Reviewed-by: Sabrina Dubroca Signed-off-by: David S. Miller Conflicts: include/net/tls.h net/tls/tls_sw.c Signed-off-by: Ziyang Xuan (cherry picked from commit 6267db2a34d34c8373ca08cc1ddd4e711daa6de9) --- include/net/tls.h | 5 ----- net/tls/tls_sw.c | 41 ++++++++--------------------------------- 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index 6654cb041693..4b61e0ea397c 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -137,9 +137,6 @@ struct tls_sw_context_tx { struct tls_rec *open_rec; struct list_head tx_list; atomic_t encrypt_pending; - /* protect crypto_wait with encrypt_pending */ - spinlock_t encrypt_compl_lock; - int async_notify; u8 async_capable:1; #define BIT_TX_SCHEDULED 0 @@ -159,8 +156,6 @@ struct tls_sw_context_rx { u8 async_capable:1; u8 decrypted:1; atomic_t decrypt_pending; - /* protect crypto_wait with decrypt_pending*/ - spinlock_t decrypt_compl_lock; }; struct tls_record_info { diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 718f682fc7ed..b14649c56d1d 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -215,22 +215,15 @@ static void tls_decrypt_done(struct crypto_async_request *req, int err) kfree(aead_req); - spin_lock_bh(&ctx->decrypt_compl_lock); - if (!atomic_dec_return(&ctx->decrypt_pending)) + if (atomic_dec_and_test(&ctx->decrypt_pending)) complete(&ctx->async_wait.completion); - spin_unlock_bh(&ctx->decrypt_compl_lock); } static int tls_decrypt_async_wait(struct tls_sw_context_rx *ctx) { - int pending; - - spin_lock_bh(&ctx->decrypt_compl_lock); - reinit_completion(&ctx->async_wait.completion); - pending = atomic_read(&ctx->decrypt_pending); - spin_unlock_bh(&ctx->decrypt_compl_lock); - if (pending) + if (!atomic_dec_and_test(&ctx->decrypt_pending)) crypto_wait_req(-EINPROGRESS, &ctx->async_wait); + atomic_inc(&ctx->decrypt_pending); return ctx->async_wait.err; } @@ -455,7 +448,6 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) struct sk_msg *msg_en; struct tls_rec *rec; bool ready = false; - int pending; rec = container_of(aead_req, struct tls_rec, aead_req); msg_en = &rec->msg_encrypted; @@ -490,12 +482,8 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) ready = true; } - spin_lock_bh(&ctx->encrypt_compl_lock); - pending = atomic_dec_return(&ctx->encrypt_pending); - - if (!pending && ctx->async_notify) + if (atomic_dec_and_test(&ctx->encrypt_pending)) complete(&ctx->async_wait.completion); - spin_unlock_bh(&ctx->encrypt_compl_lock); if (!ready) return; @@ -507,22 +495,9 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) static int tls_encrypt_async_wait(struct tls_sw_context_tx *ctx) { - int pending; - - spin_lock_bh(&ctx->encrypt_compl_lock); - ctx->async_notify = true; - - pending = atomic_read(&ctx->encrypt_pending); - spin_unlock_bh(&ctx->encrypt_compl_lock); - if (pending) + if (!atomic_dec_and_test(&ctx->encrypt_pending)) crypto_wait_req(-EINPROGRESS, &ctx->async_wait); - else - reinit_completion(&ctx->async_wait.completion); - - /* There can be no concurrent accesses, since we have no - * pending encrypt operations - */ - WRITE_ONCE(ctx->async_notify, false); + atomic_inc(&ctx->encrypt_pending); return ctx->async_wait.err; } @@ -2372,7 +2347,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx) if (tx) { crypto_init_wait(&sw_ctx_tx->async_wait); - spin_lock_init(&sw_ctx_tx->encrypt_compl_lock); + atomic_set(&sw_ctx_tx->encrypt_pending, 1); crypto_info = &ctx->crypto_send.info; cctx = &ctx->tx; aead = &sw_ctx_tx->aead_send; @@ -2381,7 +2356,7 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx) sw_ctx_tx->tx_work.sk = sk; } else { crypto_init_wait(&sw_ctx_rx->async_wait); - spin_lock_init(&sw_ctx_rx->decrypt_compl_lock); + atomic_set(&sw_ctx_rx->decrypt_pending, 1); crypto_info = &ctx->crypto_recv.info; cctx = &ctx->rx; skb_queue_head_init(&sw_ctx_rx->rx_list); -- Gitee From 5228123d60c3bb779091fbabad942e3767e23ea9 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Thu, 29 Feb 2024 14:36:45 +0800 Subject: [PATCH 4/4] tls: fix race between tx work scheduling and socket close mainline inclusion from mainline-v6.8-rc5 commit e01e3934a1b2d122919f73bc6ddbe1cdafc4bbdb category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I92REK CVE: CVE-2024-26583 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e01e3934a1b2d122919f73bc6ddbe1cdafc4bbdb -------------------------------- Similarly to previous commit, the submitting thread (recvmsg/sendmsg) may exit as soon as the async crypto handler calls complete(). Reorder scheduling the work before calling complete(). This seems more logical in the first place, as it's the inverse order of what the submitting thread will do. Reported-by: valis Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance") Signed-off-by: Jakub Kicinski Reviewed-by: Simon Horman Reviewed-by: Sabrina Dubroca Signed-off-by: David S. Miller Conflicts: net/tls/tls_sw.c Signed-off-by: Ziyang Xuan (cherry picked from commit d2631e6150bbc95a581061502aa7eb3bbbb186a3) --- net/tls/tls_sw.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index b14649c56d1d..33cd1f99fea6 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -447,7 +447,6 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) struct scatterlist *sge; struct sk_msg *msg_en; struct tls_rec *rec; - bool ready = false; rec = container_of(aead_req, struct tls_rec, aead_req); msg_en = &rec->msg_encrypted; @@ -478,19 +477,16 @@ static void tls_encrypt_done(struct crypto_async_request *req, int err) /* If received record is at head of tx_list, schedule tx */ first_rec = list_first_entry(&ctx->tx_list, struct tls_rec, list); - if (rec == first_rec) - ready = true; + if (rec == first_rec) { + /* Schedule the transmission */ + if (!test_and_set_bit(BIT_TX_SCHEDULED, + &ctx->tx_bitmask)) + schedule_delayed_work(&ctx->tx_work.work, 1); + } } if (atomic_dec_and_test(&ctx->encrypt_pending)) complete(&ctx->async_wait.completion); - - if (!ready) - return; - - /* Schedule the transmission */ - if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask)) - schedule_delayed_work(&ctx->tx_work.work, 1); } static int tls_encrypt_async_wait(struct tls_sw_context_tx *ctx) -- Gitee