[PATCH] smb: client: fix use-after-free in crypt_message when using async crypto

Wang Zhaolong posted 1 patch 3 months ago
fs/smb/client/smb2ops.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] smb: client: fix use-after-free in crypt_message when using async crypto
Posted by Wang Zhaolong 3 months ago
The CVE-2024-50047 fix removed asynchronous crypto handling from
crypt_message(), assuming all crypto operations are synchronous.
However, when hardware crypto accelerators are used, this can cause
use-after-free crashes:

  crypt_message()
    // Allocate the creq buffer containing the req
    creq = smb2_get_aead_req(..., &req);

    // Async encryption returns -EINPROGRESS immediately
    rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);

    // Free creq while async operation is still in progress
    kvfree_sensitive(creq, ...);

Hardware crypto modules often implement async AEAD operations for
performance. When crypto_aead_encrypt/decrypt() returns -EINPROGRESS,
the operation completes asynchronously. Without crypto_wait_req(),
the function immediately frees the request buffer, leading to crashes
when the driver later accesses the freed memory.

This results in a use-after-free condition when the hardware crypto
driver later accesses the freed request structure, leading to kernel
crashes with NULL pointer dereferences.

The issue occurs because crypto_alloc_aead() with mask=0 doesn't
guarantee synchronous operation. Even without CRYPTO_ALG_ASYNC in
the mask, async implementations can be selected.

Fix by restoring the async crypto handling:
- DECLARE_CRYPTO_WAIT(wait) for completion tracking
- aead_request_set_callback() for async completion notification
- crypto_wait_req() to wait for operation completion

This ensures the request buffer isn't freed until the crypto operation
completes, whether synchronous or asynchronous, while preserving the
CVE-2024-50047 fix.

Fixes: b0abcd65ec54 ("smb: client: fix UAF in async decryption")
Link: https://lore.kernel.org/all/8b784a13-87b0-4131-9ff9-7a8993538749@huaweicloud.com/
Cc: stable@vger.kernel.org
Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
---
 fs/smb/client/smb2ops.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 1468c16ea9b8..cb659256d219 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4314,10 +4314,11 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 	struct scatterlist *sg;
 	u8 sign[SMB2_SIGNATURE_SIZE] = {};
 	u8 key[SMB3_ENC_DEC_KEY_SIZE];
 	struct aead_request *req;
 	u8 *iv;
+	DECLARE_CRYPTO_WAIT(wait);
 	unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
 	void *creq;
 	size_t sensitive_size;
 
 	rc = smb2_get_enc_key(server, le64_to_cpu(tr_hdr->SessionId), enc, key);
@@ -4364,11 +4365,15 @@ crypt_message(struct TCP_Server_Info *server, int num_rqst,
 
 	aead_request_set_tfm(req, tfm);
 	aead_request_set_crypt(req, sg, sg, crypt_len, iv);
 	aead_request_set_ad(req, assoc_data_len);
 
-	rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+				  crypto_req_done, &wait);
+
+	rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+				: crypto_aead_decrypt(req), &wait);
 
 	if (!rc && enc)
 		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
 
 	kvfree_sensitive(creq, sensitive_size);
-- 
2.34.3
Re: [PATCH] smb: client: fix use-after-free in crypt_message when using async crypto
Posted by Paulo Alcantara 2 months, 4 weeks ago
Wang Zhaolong <wangzhaolong@huaweicloud.com> writes:

> The CVE-2024-50047 fix removed asynchronous crypto handling from
> crypt_message(), assuming all crypto operations are synchronous.
> However, when hardware crypto accelerators are used, this can cause
> use-after-free crashes:
>
>   crypt_message()
>     // Allocate the creq buffer containing the req
>     creq = smb2_get_aead_req(..., &req);
>
>     // Async encryption returns -EINPROGRESS immediately
>     rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
>
>     // Free creq while async operation is still in progress
>     kvfree_sensitive(creq, ...);
>
> Hardware crypto modules often implement async AEAD operations for
> performance. When crypto_aead_encrypt/decrypt() returns -EINPROGRESS,
> the operation completes asynchronously. Without crypto_wait_req(),
> the function immediately frees the request buffer, leading to crashes
> when the driver later accesses the freed memory.
>
> This results in a use-after-free condition when the hardware crypto
> driver later accesses the freed request structure, leading to kernel
> crashes with NULL pointer dereferences.
>
> The issue occurs because crypto_alloc_aead() with mask=0 doesn't
> guarantee synchronous operation. Even without CRYPTO_ALG_ASYNC in
> the mask, async implementations can be selected.
>
> Fix by restoring the async crypto handling:
> - DECLARE_CRYPTO_WAIT(wait) for completion tracking
> - aead_request_set_callback() for async completion notification
> - crypto_wait_req() to wait for operation completion
>
> This ensures the request buffer isn't freed until the crypto operation
> completes, whether synchronous or asynchronous, while preserving the
> CVE-2024-50047 fix.
>
> Fixes: b0abcd65ec54 ("smb: client: fix UAF in async decryption")
> Link: https://lore.kernel.org/all/8b784a13-87b0-4131-9ff9-7a8993538749@huaweicloud.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>  fs/smb/client/smb2ops.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>