From patchwork Mon Mar 25 10:32:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juerg Haefliger X-Patchwork-Id: 1915507 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4V38RP5bWZz1yWy for ; Mon, 25 Mar 2024 21:33:37 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1rohdu-0000Sq-Io; Mon, 25 Mar 2024 10:33:26 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1rohdb-0000Ks-W4 for kernel-team@lists.ubuntu.com; Mon, 25 Mar 2024 10:33:08 +0000 Received: from mail-lj1-f198.google.com (mail-lj1-f198.google.com [209.85.208.198]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id 769BE3F204 for ; Mon, 25 Mar 2024 10:33:07 +0000 (UTC) Received: by mail-lj1-f198.google.com with SMTP id 38308e7fff4ca-2d6b41203b2so21728041fa.1 for ; Mon, 25 Mar 2024 03:33:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1711362787; x=1711967587; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=IMGO383sqelszIhBtJaUggQxAsS2lgYFEh1CFYtHUzo=; b=lWyHx+zu5eGvcIsfwYwIbW6FxfQ0EUEZkVKWJWFkHP6thfanl9cSGkZRwtCXUKovEe I3/GyqMqGbH+Hx16wm/GLDytmMhVgHaOz1KW/D1l8VUoEkx1D32rNKcJsr4PS+L1p4yk 797+hm9/dxdzlftP2eFPnVvZDkN3ReFJsPLFoCWWPiLSfdvk/5gByuLHC1qB6D78Vh+S 0GvRNbhITWWWOf5h5/k+lIJW82nImbXhERsN7gYB7eyORsPLpzYDW6IwP7rf5K8rB3q8 DDztZ7Mc2eOpyEE2VW7Q8wrfIe+qZXtsQwTKTvKtl9nT2yk73a6xITkDyOpHB1TnKWyP NiXQ== X-Gm-Message-State: AOJu0YysQzO/YcuATC1xtYixmIZ5D5c2WwNsjZAnKgk2RI2DsaeRRYDM DjNEdIYEabg7z9OBjqCOeICS/PZ6NBVFwhn7rszS0aiR0eJkoPWkCMXttTjmTT2Ki4QbqkS0f4N 88tEDJzRAToyGDxiwSn0FljZZi6hVUnd5lg6LQwvRi1cxiJpiZi0OMzBUWw1q0DzBeSvmcgcc7Q lzXK+b3BV+1A== X-Received: by 2002:a2e:9001:0:b0:2d4:720f:6f46 with SMTP id h1-20020a2e9001000000b002d4720f6f46mr3900907ljg.15.1711362786887; Mon, 25 Mar 2024 03:33:06 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHztPOl7gFCVXzxiTUtKgF14l5ZrsZt2yksOCyw/en+VhpIBqLbGlLdAzlTdbiV1dP3F5FHHw== X-Received: by 2002:a2e:9001:0:b0:2d4:720f:6f46 with SMTP id h1-20020a2e9001000000b002d4720f6f46mr3900899ljg.15.1711362786608; Mon, 25 Mar 2024 03:33:06 -0700 (PDT) Received: from localhost ([81.221.247.52]) by smtp.gmail.com with ESMTPSA id q2-20020adfcd82000000b0033e7603987dsm9185738wrj.12.2024.03.25.03.33.06 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 25 Mar 2024 03:33:06 -0700 (PDT) From: Juerg Haefliger To: kernel-team@lists.ubuntu.com Subject: [SRU][M][PATCH 4/8] tls: fix race between async notify and socket close Date: Mon, 25 Mar 2024 11:32:56 +0100 Message-Id: <20240325103300.494141-5-juerg.haefliger@canonical.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240325103300.494141-1-juerg.haefliger@canonical.com> References: <20240325103300.494141-1-juerg.haefliger@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jakub Kicinski CVE-2024-26583 [ Upstream commit 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 Signed-off-by: Sasha Levin (cherry picked from commit 86dc27ee36f558fe223dbdfbfcb6856247356f4a linux-6.6.y) Signed-off-by: Juerg Haefliger --- include/net/tls.h | 5 ----- net/tls/tls_sw.c | 43 ++++++++++--------------------------------- 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/include/net/tls.h b/include/net/tls.h index 5e71dd3df8ca..1815a4ee79e7 100644 --- a/include/net/tls.h +++ b/include/net/tls.h @@ -110,9 +110,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 @@ -149,8 +146,6 @@ struct tls_sw_context_rx { struct tls_strparser strp; atomic_t decrypt_pending; - /* protect crypto_wait with decrypt_pending*/ - spinlock_t decrypt_compl_lock; struct sk_buff_head async_hold; struct wait_queue_head wq; }; diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index 63060bb1b5ff..aaa97be47387 100644 --- a/net/tls/tls_sw.c +++ b/net/tls/tls_sw.c @@ -224,22 +224,15 @@ static void tls_decrypt_done(void *data, 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; } @@ -267,6 +260,7 @@ static int tls_do_decryption(struct sock *sk, aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG, tls_decrypt_done, aead_req); + DEBUG_NET_WARN_ON_ONCE(atomic_read(&ctx->decrypt_pending) < 1); atomic_inc(&ctx->decrypt_pending); } else { aead_request_set_callback(aead_req, @@ -455,7 +449,6 @@ static void tls_encrypt_done(void *data, int err) struct sk_msg *msg_en; bool ready = false; struct sock *sk; - int pending; msg_en = &rec->msg_encrypted; @@ -494,12 +487,8 @@ static void tls_encrypt_done(void *data, 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; @@ -511,22 +500,9 @@ static void tls_encrypt_done(void *data, 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; } @@ -577,6 +553,7 @@ static int tls_do_encryption(struct sock *sk, /* Add the record in tx_list */ list_add_tail((struct list_head *)&rec->list, &ctx->tx_list); + DEBUG_NET_WARN_ON_ONCE(atomic_read(&ctx->encrypt_pending) < 1); atomic_inc(&ctx->encrypt_pending); rc = crypto_aead_encrypt(aead_req); @@ -2505,7 +2482,7 @@ static struct tls_sw_context_tx *init_ctx_tx(struct tls_context *ctx, struct soc } 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); INIT_LIST_HEAD(&sw_ctx_tx->tx_list); INIT_DELAYED_WORK(&sw_ctx_tx->tx_work.work, tx_work_handler); sw_ctx_tx->tx_work.sk = sk; @@ -2526,7 +2503,7 @@ static struct tls_sw_context_rx *init_ctx_rx(struct tls_context *ctx) } 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); init_waitqueue_head(&sw_ctx_rx->wq); skb_queue_head_init(&sw_ctx_rx->rx_list); skb_queue_head_init(&sw_ctx_rx->async_hold);