Skip to content

Commit 88f1708

Browse files
Marios Makassikissmfrench
authored andcommitted
ksmbd: fix recursive locking in RPC handle list access
Since commit 305853c ("ksmbd: Fix race condition in RPC handle list access"), ksmbd_session_rpc_method() attempts to lock sess->rpc_lock. This causes hung connections / tasks when a client attempts to open a named pipe. Using Samba's rpcclient tool: $ rpcclient //192.168.1.254 -U user%password $ rpcclient $> srvinfo <connection hung here> Kernel side: "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:kworker/0:0 state:D stack:0 pid:5021 tgid:5021 ppid:2 flags:0x00200000 Workqueue: ksmbd-io handle_ksmbd_work Call trace: __schedule from schedule+0x3c/0x58 schedule from schedule_preempt_disabled+0xc/0x10 schedule_preempt_disabled from rwsem_down_read_slowpath+0x1b0/0x1d8 rwsem_down_read_slowpath from down_read+0x28/0x30 down_read from ksmbd_session_rpc_method+0x18/0x3c ksmbd_session_rpc_method from ksmbd_rpc_open+0x34/0x68 ksmbd_rpc_open from ksmbd_session_rpc_open+0x194/0x228 ksmbd_session_rpc_open from create_smb2_pipe+0x8c/0x2c8 create_smb2_pipe from smb2_open+0x10c/0x27ac smb2_open from handle_ksmbd_work+0x238/0x3dc handle_ksmbd_work from process_scheduled_works+0x160/0x25c process_scheduled_works from worker_thread+0x16c/0x1e8 worker_thread from kthread+0xa8/0xb8 kthread from ret_from_fork+0x14/0x38 Exception stack(0x8529ffb0 to 0x8529fff8) The task deadlocks because the lock is already held: ksmbd_session_rpc_open down_write(&sess->rpc_lock) ksmbd_rpc_open ksmbd_session_rpc_method down_read(&sess->rpc_lock) <-- deadlock Adjust ksmbd_session_rpc_method() callers to take the lock when necessary. Fixes: 305853c ("ksmbd: Fix race condition in RPC handle list access") Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 379510a commit 88f1708

File tree

3 files changed

+22
-6
lines changed

3 files changed

+22
-6
lines changed

fs/smb/server/mgmt/user_session.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,14 +147,11 @@ void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id)
147147
int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id)
148148
{
149149
struct ksmbd_session_rpc *entry;
150-
int method;
151150

152-
down_read(&sess->rpc_lock);
151+
lockdep_assert_held(&sess->rpc_lock);
153152
entry = xa_load(&sess->rpc_handle_list, id);
154-
method = entry ? entry->method : 0;
155-
up_read(&sess->rpc_lock);
156153

157-
return method;
154+
return entry ? entry->method : 0;
158155
}
159156

160157
void ksmbd_session_destroy(struct ksmbd_session *sess)

fs/smb/server/smb2pdu.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4626,8 +4626,15 @@ static int smb2_get_info_file_pipe(struct ksmbd_session *sess,
46264626
* pipe without opening it, checking error condition here
46274627
*/
46284628
id = req->VolatileFileId;
4629-
if (!ksmbd_session_rpc_method(sess, id))
4629+
4630+
lockdep_assert_not_held(&sess->rpc_lock);
4631+
4632+
down_read(&sess->rpc_lock);
4633+
if (!ksmbd_session_rpc_method(sess, id)) {
4634+
up_read(&sess->rpc_lock);
46304635
return -ENOENT;
4636+
}
4637+
up_read(&sess->rpc_lock);
46314638

46324639
ksmbd_debug(SMB, "FileInfoClass %u, FileId 0x%llx\n",
46334640
req->FileInfoClass, req->VolatileFileId);

fs/smb/server/transport_ipc.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,9 @@ struct ksmbd_rpc_command *ksmbd_rpc_write(struct ksmbd_session *sess, int handle
825825
if (!msg)
826826
return NULL;
827827

828+
lockdep_assert_not_held(&sess->rpc_lock);
829+
830+
down_read(&sess->rpc_lock);
828831
msg->type = KSMBD_EVENT_RPC_REQUEST;
829832
req = (struct ksmbd_rpc_command *)msg->payload;
830833
req->handle = handle;
@@ -833,6 +836,7 @@ struct ksmbd_rpc_command *ksmbd_rpc_write(struct ksmbd_session *sess, int handle
833836
req->flags |= KSMBD_RPC_WRITE_METHOD;
834837
req->payload_sz = payload_sz;
835838
memcpy(req->payload, payload, payload_sz);
839+
up_read(&sess->rpc_lock);
836840

837841
resp = ipc_msg_send_request(msg, req->handle);
838842
ipc_msg_free(msg);
@@ -849,13 +853,17 @@ struct ksmbd_rpc_command *ksmbd_rpc_read(struct ksmbd_session *sess, int handle)
849853
if (!msg)
850854
return NULL;
851855

856+
lockdep_assert_not_held(&sess->rpc_lock);
857+
858+
down_read(&sess->rpc_lock);
852859
msg->type = KSMBD_EVENT_RPC_REQUEST;
853860
req = (struct ksmbd_rpc_command *)msg->payload;
854861
req->handle = handle;
855862
req->flags = ksmbd_session_rpc_method(sess, handle);
856863
req->flags |= rpc_context_flags(sess);
857864
req->flags |= KSMBD_RPC_READ_METHOD;
858865
req->payload_sz = 0;
866+
up_read(&sess->rpc_lock);
859867

860868
resp = ipc_msg_send_request(msg, req->handle);
861869
ipc_msg_free(msg);
@@ -876,6 +884,9 @@ struct ksmbd_rpc_command *ksmbd_rpc_ioctl(struct ksmbd_session *sess, int handle
876884
if (!msg)
877885
return NULL;
878886

887+
lockdep_assert_not_held(&sess->rpc_lock);
888+
889+
down_read(&sess->rpc_lock);
879890
msg->type = KSMBD_EVENT_RPC_REQUEST;
880891
req = (struct ksmbd_rpc_command *)msg->payload;
881892
req->handle = handle;
@@ -884,6 +895,7 @@ struct ksmbd_rpc_command *ksmbd_rpc_ioctl(struct ksmbd_session *sess, int handle
884895
req->flags |= KSMBD_RPC_IOCTL_METHOD;
885896
req->payload_sz = payload_sz;
886897
memcpy(req->payload, payload, payload_sz);
898+
up_read(&sess->rpc_lock);
887899

888900
resp = ipc_msg_send_request(msg, req->handle);
889901
ipc_msg_free(msg);

0 commit comments

Comments
 (0)