Re: [PATCH] ksmbd: ipc: fix use-after-free in ipc_msg_send_request

[Namjae Jeon]

On Wed, Nov 26, 2025 at 10:49 AM Qianchang Zhao <pioooooooooip@xxxxxxxxx> 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@xxxxxxxxx>
> Reported-by: Zhitong Liu <liuzhitong1993@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Qianchang Zhao <pioooooooooip@xxxxxxxxx>
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!
From 3858665313f1f0e1c09934418b43ea6337dd5b7f Mon Sep 17 00:00:00 2001
From: Qianchang Zhao <pioooooooooip@xxxxxxxxx>
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@xxxxxxxxxxxxxxx
Reported-by: Qianchang Zhao <pioooooooooip@xxxxxxxxx>
Reported-by: Zhitong Liu <liuzhitong1993@xxxxxxxxx>
Signed-off-by: Qianchang Zhao <pioooooooooip@xxxxxxxxx>
Acked-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
 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