public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request
       [not found] <CAKYAXd8=buKQRve+pdBFp9ce+5MiR02ZnHtGHy-hYDfhGWn=pQ@mail.gmail.com>
@ 2025-11-26  1:49 ` Qianchang Zhao
  2025-11-26  3:28   ` Namjae Jeon
  0 siblings, 1 reply; 2+ messages in thread
From: Qianchang Zhao @ 2025-11-26  1:49 UTC (permalink / raw)
  To: Namjae Jeon, Steve French
  Cc: gregkh, linux-cifs, linux-kernel, Zhitong Liu, Qianchang Zhao,
	stable

ipc_msg_send_request() waits for a generic netlink reply using an
ipc_msg_table_entry on the stack. The generic netlink handler
(handle_generic_event()/handle_response()) fills entry->response under
ipc_msg_table_lock, but ipc_msg_send_request() used to validate and free
entry->response without holding the same lock.

Under high concurrency this allows a race where handle_response() is
copying data into entry->response while ipc_msg_send_request() has just
freed it, leading to a slab-use-after-free reported by KASAN in
handle_generic_event():

  BUG: KASAN: slab-use-after-free in handle_generic_event+0x3c4/0x5f0 [ksmbd]
  Write of size 12 at addr ffff888198ee6e20 by task pool/109349
  ...
  Freed by task:
    kvfree
    ipc_msg_send_request [ksmbd]
    ksmbd_rpc_open -> ksmbd_session_rpc_open [ksmbd]

Fix by:
- Taking ipc_msg_table_lock in ipc_msg_send_request() while validating
  entry->response, freeing it when invalid, and removing the entry from
  ipc_msg_table.
- Returning the final entry->response pointer to the caller only after
  the hash entry is removed under the lock.
- Returning NULL in the error path, preserving the original API
  semantics.

This makes all accesses to entry->response consistent with
handle_response(), which already updates and fills the response buffer
under ipc_msg_table_lock, and closes the race that allowed the UAF.

Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Qianchang Zhao <pioooooooooip@gmail.com>
---
 fs/smb/server/transport_ipc.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/transport_ipc.c b/fs/smb/server/transport_ipc.c
index 46f87fd1c..7b1a060da 100644
--- a/fs/smb/server/transport_ipc.c
+++ b/fs/smb/server/transport_ipc.c
@@ -532,6 +532,7 @@ static int ipc_validate_msg(struct ipc_msg_table_entry *entry)
 static void *ipc_msg_send_request(struct ksmbd_ipc_msg *msg, unsigned int handle)
 {
 	struct ipc_msg_table_entry entry;
+	void *response = NULL;
 	int ret;
 
 	if ((int)handle < 0)
@@ -553,6 +554,8 @@ static void *ipc_msg_send_request(struct ksmbd_ipc_msg *msg, unsigned int handle
 	ret = wait_event_interruptible_timeout(entry.wait,
 					       entry.response != NULL,
 					       IPC_WAIT_TIMEOUT);
+
+	down_write(&ipc_msg_table_lock);
 	if (entry.response) {
 		ret = ipc_validate_msg(&entry);
 		if (ret) {
@@ -560,11 +563,19 @@ static void *ipc_msg_send_request(struct ksmbd_ipc_msg *msg, unsigned int handle
 			entry.response = NULL;
 		}
 	}
+
+	response = entry.response;
+	hash_del(&entry.ipc_table_hlist);
+	up_write(&ipc_msg_table_lock);
+
+	return response;
+
 out:
 	down_write(&ipc_msg_table_lock);
 	hash_del(&entry.ipc_table_hlist);
 	up_write(&ipc_msg_table_lock);
-	return entry.response;
+
+	return NULL;
 }
 
 static int ksmbd_ipc_heartbeat_request(void)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request
  2025-11-26  1:49 ` [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request Qianchang Zhao
@ 2025-11-26  3:28   ` Namjae Jeon
  0 siblings, 0 replies; 2+ messages in thread
From: Namjae Jeon @ 2025-11-26  3:28 UTC (permalink / raw)
  To: Qianchang Zhao
  Cc: Steve French, gregkh, linux-cifs, linux-kernel, Zhitong Liu,
	stable

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Wed, Nov 26, 2025 at 10:49 AM Qianchang Zhao <pioooooooooip@gmail.com> wrote:
>
> ipc_msg_send_request() waits for a generic netlink reply using an
> ipc_msg_table_entry on the stack. The generic netlink handler
> (handle_generic_event()/handle_response()) fills entry->response under
> ipc_msg_table_lock, but ipc_msg_send_request() used to validate and free
> entry->response without holding the same lock.
>
> Under high concurrency this allows a race where handle_response() is
> copying data into entry->response while ipc_msg_send_request() has just
> freed it, leading to a slab-use-after-free reported by KASAN in
> handle_generic_event():
>
>   BUG: KASAN: slab-use-after-free in handle_generic_event+0x3c4/0x5f0 [ksmbd]
>   Write of size 12 at addr ffff888198ee6e20 by task pool/109349
>   ...
>   Freed by task:
>     kvfree
>     ipc_msg_send_request [ksmbd]
>     ksmbd_rpc_open -> ksmbd_session_rpc_open [ksmbd]
>
> Fix by:
> - Taking ipc_msg_table_lock in ipc_msg_send_request() while validating
>   entry->response, freeing it when invalid, and removing the entry from
>   ipc_msg_table.
> - Returning the final entry->response pointer to the caller only after
>   the hash entry is removed under the lock.
> - Returning NULL in the error path, preserving the original API
>   semantics.
>
> This makes all accesses to entry->response consistent with
> handle_response(), which already updates and fills the response buffer
> under ipc_msg_table_lock, and closes the race that allowed the UAF.
>
> Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
> Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Qianchang Zhao <pioooooooooip@gmail.com>
I have directly updated your patch and applied it to #ksmbd-for-next-next.
Let me know if the attached patch has some issue.
Thanks!

[-- Attachment #2: 0001-ksmbd-ipc-fix-use-after-free-in-ipc_msg_send_request.patch --]
[-- Type: text/x-patch, Size: 2907 bytes --]

From 3858665313f1f0e1c09934418b43ea6337dd5b7f Mon Sep 17 00:00:00 2001
From: Qianchang Zhao <pioooooooooip@gmail.com>
Date: Wed, 26 Nov 2025 12:24:18 +0900
Subject: [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request

ipc_msg_send_request() waits for a generic netlink reply using an
ipc_msg_table_entry on the stack. The generic netlink handler
(handle_generic_event()/handle_response()) fills entry->response under
ipc_msg_table_lock, but ipc_msg_send_request() used to validate and free
entry->response without holding the same lock.

Under high concurrency this allows a race where handle_response() is
copying data into entry->response while ipc_msg_send_request() has just
freed it, leading to a slab-use-after-free reported by KASAN in
handle_generic_event():

  BUG: KASAN: slab-use-after-free in handle_generic_event+0x3c4/0x5f0 [ksmbd]
  Write of size 12 at addr ffff888198ee6e20 by task pool/109349
  ...
  Freed by task:
    kvfree
    ipc_msg_send_request [ksmbd]
    ksmbd_rpc_open -> ksmbd_session_rpc_open [ksmbd]

Fix by:
- Taking ipc_msg_table_lock in ipc_msg_send_request() while validating
  entry->response, freeing it when invalid, and removing the entry from
  ipc_msg_table.
- Returning the final entry->response pointer to the caller only after
  the hash entry is removed under the lock.
- Returning NULL in the error path, preserving the original API
  semantics.

This makes all accesses to entry->response consistent with
handle_response(), which already updates and fills the response buffer
under ipc_msg_table_lock, and closes the race that allowed the UAF.

Cc: stable@vger.kernel.org
Reported-by: Qianchang Zhao <pioooooooooip@gmail.com>
Reported-by: Zhitong Liu <liuzhitong1993@gmail.com>
Signed-off-by: Qianchang Zhao <pioooooooooip@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/server/transport_ipc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/smb/server/transport_ipc.c b/fs/smb/server/transport_ipc.c
index 2c08cccfa680..2dbabe2d8005 100644
--- a/fs/smb/server/transport_ipc.c
+++ b/fs/smb/server/transport_ipc.c
@@ -553,12 +553,16 @@ static void *ipc_msg_send_request(struct ksmbd_ipc_msg *msg, unsigned int handle
 	up_write(&ipc_msg_table_lock);
 
 	ret = ipc_msg_send(msg);
-	if (ret)
+	if (ret) {
+		down_write(&ipc_msg_table_lock);
 		goto out;
+	}
 
 	ret = wait_event_interruptible_timeout(entry.wait,
 					       entry.response != NULL,
 					       IPC_WAIT_TIMEOUT);
+
+	down_write(&ipc_msg_table_lock);
 	if (entry.response) {
 		ret = ipc_validate_msg(&entry);
 		if (ret) {
@@ -567,7 +571,6 @@ static void *ipc_msg_send_request(struct ksmbd_ipc_msg *msg, unsigned int handle
 		}
 	}
 out:
-	down_write(&ipc_msg_table_lock);
 	hash_del(&entry.ipc_table_hlist);
 	up_write(&ipc_msg_table_lock);
 	return entry.response;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-11-26  3:28 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAKYAXd8=buKQRve+pdBFp9ce+5MiR02ZnHtGHy-hYDfhGWn=pQ@mail.gmail.com>
2025-11-26  1:49 ` [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request Qianchang Zhao
2025-11-26  3:28   ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox