From: Jakub Kicinski kuba@kernel.org
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/?i...
--------------------------------
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 sec@valis.email Fixes: 0cada33241d9 ("net/tls: fix race condition causing kernel panic") Signed-off-by: Jakub Kicinski kuba@kernel.org Reviewed-by: Simon Horman horms@kernel.org Reviewed-by: Eric Dumazet edumazet@google.com Reviewed-by: Sabrina Dubroca sd@queasysnail.net Signed-off-by: David S. Miller davem@davemloft.net Conflicts: include/net/tls.h net/tls/tls_sw.c Signed-off-by: Ziyang Xuan william.xuanziyang@huawei.com --- 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 18b336e4cbbc..7242e9cdd5ad 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; } @@ -2386,7 +2361,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; @@ -2395,7 +2370,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);