Skip to content

Commit 5cc679b

Browse files
kzall0cgregkh
authored andcommitted
ksmbd: Fix race condition in RPC handle list access
commit 305853c upstream. The 'sess->rpc_handle_list' XArray manages RPC handles within a ksmbd session. Access to this list is intended to be protected by 'sess->rpc_lock' (an rw_semaphore). However, the locking implementation was flawed, leading to potential race conditions. In ksmbd_session_rpc_open(), the code incorrectly acquired only a read lock before calling xa_store() and xa_erase(). Since these operations modify the XArray structure, a write lock is required to ensure exclusive access and prevent data corruption from concurrent modifications. Furthermore, ksmbd_session_rpc_method() accessed the list using xa_load() without holding any lock at all. This could lead to reading inconsistent data or a potential use-after-free if an entry is concurrently removed and the pointer is dereferenced. Fix these issues by: 1. Using down_write() and up_write() in ksmbd_session_rpc_open() to ensure exclusive access during XArray modification, and ensuring the lock is correctly released on error paths. 2. Adding down_read() and up_read() in ksmbd_session_rpc_method() to safely protect the lookup. Fixes: a1f46c9 ("ksmbd: fix use-after-free in ksmbd_session_rpc_open") Fixes: b685757 ("ksmbd: Implements sess->rpc_handle_list as xarray") Cc: stable@vger.kernel.org Signed-off-by: Yunseong Kim <ysk@kzalloc.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 987f2bd commit 5cc679b

File tree

1 file changed

+17
-9
lines changed

1 file changed

+17
-9
lines changed

fs/smb/server/mgmt/user_session.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -104,29 +104,32 @@ int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name)
104104
if (!entry)
105105
return -ENOMEM;
106106

107-
down_read(&sess->rpc_lock);
108107
entry->method = method;
109108
entry->id = id = ksmbd_ipc_id_alloc();
110109
if (id < 0)
111110
goto free_entry;
111+
112+
down_write(&sess->rpc_lock);
112113
old = xa_store(&sess->rpc_handle_list, id, entry, KSMBD_DEFAULT_GFP);
113-
if (xa_is_err(old))
114+
if (xa_is_err(old)) {
115+
up_write(&sess->rpc_lock);
114116
goto free_id;
117+
}
115118

116119
resp = ksmbd_rpc_open(sess, id);
117-
if (!resp)
118-
goto erase_xa;
120+
if (!resp) {
121+
xa_erase(&sess->rpc_handle_list, entry->id);
122+
up_write(&sess->rpc_lock);
123+
goto free_id;
124+
}
119125

120-
up_read(&sess->rpc_lock);
126+
up_write(&sess->rpc_lock);
121127
kvfree(resp);
122128
return id;
123-
erase_xa:
124-
xa_erase(&sess->rpc_handle_list, entry->id);
125129
free_id:
126130
ksmbd_rpc_id_free(entry->id);
127131
free_entry:
128132
kfree(entry);
129-
up_read(&sess->rpc_lock);
130133
return -EINVAL;
131134
}
132135

@@ -144,9 +147,14 @@ void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id)
144147
int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id)
145148
{
146149
struct ksmbd_session_rpc *entry;
150+
int method;
147151

152+
down_read(&sess->rpc_lock);
148153
entry = xa_load(&sess->rpc_handle_list, id);
149-
return entry ? entry->method : 0;
154+
method = entry ? entry->method : 0;
155+
up_read(&sess->rpc_lock);
156+
157+
return method;
150158
}
151159

152160
void ksmbd_session_destroy(struct ksmbd_session *sess)

0 commit comments

Comments
 (0)