* 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 [flat|nested] 2+ messages in thread