Skip to content

Commit abcc506

Browse files
namjaejeonSteve French
authored andcommitted
ksmbd: fix racy issue from smb2 close and logoff with multichannel
When smb client send concurrent smb2 close and logoff request with multichannel connection, It can cause racy issue. logoff request free tcon and can cause UAF issues in smb2 close. When receiving logoff request with multichannel, ksmbd should wait until all remaning requests complete as well as ones in the current connection, and then make session expired. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-20796 ZDI-CAN-20595 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 3353ab2 commit abcc506

File tree

5 files changed

+101
-32
lines changed

5 files changed

+101
-32
lines changed

fs/ksmbd/connection.c

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ static DEFINE_MUTEX(init_lock);
2020
static struct ksmbd_conn_ops default_conn_ops;
2121

2222
LIST_HEAD(conn_list);
23-
DEFINE_RWLOCK(conn_list_lock);
23+
DECLARE_RWSEM(conn_list_lock);
2424

2525
/**
2626
* ksmbd_conn_free() - free resources of the connection instance
@@ -32,9 +32,9 @@ DEFINE_RWLOCK(conn_list_lock);
3232
*/
3333
void ksmbd_conn_free(struct ksmbd_conn *conn)
3434
{
35-
write_lock(&conn_list_lock);
35+
down_write(&conn_list_lock);
3636
list_del(&conn->conns_list);
37-
write_unlock(&conn_list_lock);
37+
up_write(&conn_list_lock);
3838

3939
xa_destroy(&conn->sessions);
4040
kvfree(conn->request_buf);
@@ -84,9 +84,9 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
8484
spin_lock_init(&conn->llist_lock);
8585
INIT_LIST_HEAD(&conn->lock_list);
8686

87-
write_lock(&conn_list_lock);
87+
down_write(&conn_list_lock);
8888
list_add(&conn->conns_list, &conn_list);
89-
write_unlock(&conn_list_lock);
89+
up_write(&conn_list_lock);
9090
return conn;
9191
}
9292

@@ -95,15 +95,15 @@ bool ksmbd_conn_lookup_dialect(struct ksmbd_conn *c)
9595
struct ksmbd_conn *t;
9696
bool ret = false;
9797

98-
read_lock(&conn_list_lock);
98+
down_read(&conn_list_lock);
9999
list_for_each_entry(t, &conn_list, conns_list) {
100100
if (memcmp(t->ClientGUID, c->ClientGUID, SMB2_CLIENT_GUID_SIZE))
101101
continue;
102102

103103
ret = true;
104104
break;
105105
}
106-
read_unlock(&conn_list_lock);
106+
up_read(&conn_list_lock);
107107
return ret;
108108
}
109109

@@ -157,9 +157,37 @@ void ksmbd_conn_unlock(struct ksmbd_conn *conn)
157157
mutex_unlock(&conn->srv_mutex);
158158
}
159159

160-
void ksmbd_conn_wait_idle(struct ksmbd_conn *conn)
160+
void ksmbd_all_conn_set_status(u64 sess_id, u32 status)
161161
{
162+
struct ksmbd_conn *conn;
163+
164+
down_read(&conn_list_lock);
165+
list_for_each_entry(conn, &conn_list, conns_list) {
166+
if (conn->binding || xa_load(&conn->sessions, sess_id))
167+
WRITE_ONCE(conn->status, status);
168+
}
169+
up_read(&conn_list_lock);
170+
}
171+
172+
void ksmbd_conn_wait_idle(struct ksmbd_conn *conn, u64 sess_id)
173+
{
174+
struct ksmbd_conn *bind_conn;
175+
162176
wait_event(conn->req_running_q, atomic_read(&conn->req_running) < 2);
177+
178+
down_read(&conn_list_lock);
179+
list_for_each_entry(bind_conn, &conn_list, conns_list) {
180+
if (bind_conn == conn)
181+
continue;
182+
183+
if ((bind_conn->binding || xa_load(&bind_conn->sessions, sess_id)) &&
184+
!ksmbd_conn_releasing(bind_conn) &&
185+
atomic_read(&bind_conn->req_running)) {
186+
wait_event(bind_conn->req_running_q,
187+
atomic_read(&bind_conn->req_running) == 0);
188+
}
189+
}
190+
up_read(&conn_list_lock);
163191
}
164192

165193
int ksmbd_conn_write(struct ksmbd_work *work)
@@ -360,10 +388,10 @@ int ksmbd_conn_handler_loop(void *p)
360388
}
361389

362390
out:
391+
ksmbd_conn_set_releasing(conn);
363392
/* Wait till all reference dropped to the Server object*/
364393
wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);
365394

366-
367395
if (IS_ENABLED(CONFIG_UNICODE))
368396
utf8_unload(conn->um);
369397
unload_nls(conn->local_nls);
@@ -407,7 +435,7 @@ static void stop_sessions(void)
407435
struct ksmbd_transport *t;
408436

409437
again:
410-
read_lock(&conn_list_lock);
438+
down_read(&conn_list_lock);
411439
list_for_each_entry(conn, &conn_list, conns_list) {
412440
struct task_struct *task;
413441

@@ -418,12 +446,12 @@ static void stop_sessions(void)
418446
task->comm, task_pid_nr(task));
419447
ksmbd_conn_set_exiting(conn);
420448
if (t->ops->shutdown) {
421-
read_unlock(&conn_list_lock);
449+
up_read(&conn_list_lock);
422450
t->ops->shutdown(t);
423-
read_lock(&conn_list_lock);
451+
down_read(&conn_list_lock);
424452
}
425453
}
426-
read_unlock(&conn_list_lock);
454+
up_read(&conn_list_lock);
427455

428456
if (!list_empty(&conn_list)) {
429457
schedule_timeout_interruptible(HZ / 10); /* 100ms */

fs/ksmbd/connection.h

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ enum {
2626
KSMBD_SESS_GOOD,
2727
KSMBD_SESS_EXITING,
2828
KSMBD_SESS_NEED_RECONNECT,
29-
KSMBD_SESS_NEED_NEGOTIATE
29+
KSMBD_SESS_NEED_NEGOTIATE,
30+
KSMBD_SESS_RELEASING
3031
};
3132

3233
struct ksmbd_stats {
@@ -140,10 +141,10 @@ struct ksmbd_transport {
140141
#define KSMBD_TCP_PEER_SOCKADDR(c) ((struct sockaddr *)&((c)->peer_addr))
141142

142143
extern struct list_head conn_list;
143-
extern rwlock_t conn_list_lock;
144+
extern struct rw_semaphore conn_list_lock;
144145

145146
bool ksmbd_conn_alive(struct ksmbd_conn *conn);
146-
void ksmbd_conn_wait_idle(struct ksmbd_conn *conn);
147+
void ksmbd_conn_wait_idle(struct ksmbd_conn *conn, u64 sess_id);
147148
struct ksmbd_conn *ksmbd_conn_alloc(void);
148149
void ksmbd_conn_free(struct ksmbd_conn *conn);
149150
bool ksmbd_conn_lookup_dialect(struct ksmbd_conn *c);
@@ -191,6 +192,11 @@ static inline bool ksmbd_conn_exiting(struct ksmbd_conn *conn)
191192
return READ_ONCE(conn->status) == KSMBD_SESS_EXITING;
192193
}
193194

195+
static inline bool ksmbd_conn_releasing(struct ksmbd_conn *conn)
196+
{
197+
return READ_ONCE(conn->status) == KSMBD_SESS_RELEASING;
198+
}
199+
194200
static inline void ksmbd_conn_set_new(struct ksmbd_conn *conn)
195201
{
196202
WRITE_ONCE(conn->status, KSMBD_SESS_NEW);
@@ -215,4 +221,11 @@ static inline void ksmbd_conn_set_exiting(struct ksmbd_conn *conn)
215221
{
216222
WRITE_ONCE(conn->status, KSMBD_SESS_EXITING);
217223
}
224+
225+
static inline void ksmbd_conn_set_releasing(struct ksmbd_conn *conn)
226+
{
227+
WRITE_ONCE(conn->status, KSMBD_SESS_RELEASING);
228+
}
229+
230+
void ksmbd_all_conn_set_status(u64 sess_id, u32 status);
218231
#endif /* __CONNECTION_H__ */

fs/ksmbd/mgmt/tree_connect.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ int ksmbd_tree_conn_session_logoff(struct ksmbd_session *sess)
129129
struct ksmbd_tree_connect *tc;
130130
unsigned long id;
131131

132+
if (!sess)
133+
return -EINVAL;
134+
132135
xa_for_each(&sess->tree_conns, id, tc)
133136
ret |= ksmbd_tree_conn_disconnect(sess, tc);
134137
xa_destroy(&sess->tree_conns);

fs/ksmbd/mgmt/user_session.c

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
144144
if (!sess)
145145
return;
146146

147-
down_write(&sessions_table_lock);
148-
hash_del(&sess->hlist);
149-
up_write(&sessions_table_lock);
150-
151147
if (sess->user)
152148
ksmbd_free_user(sess->user);
153149

@@ -178,15 +174,18 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn)
178174
unsigned long id;
179175
struct ksmbd_session *sess;
180176

177+
down_write(&sessions_table_lock);
181178
xa_for_each(&conn->sessions, id, sess) {
182179
if (sess->state != SMB2_SESSION_VALID ||
183180
time_after(jiffies,
184181
sess->last_active + SMB2_SESSION_TIMEOUT)) {
185182
xa_erase(&conn->sessions, sess->id);
183+
hash_del(&sess->hlist);
186184
ksmbd_session_destroy(sess);
187185
continue;
188186
}
189187
}
188+
up_write(&sessions_table_lock);
190189
}
191190

192191
int ksmbd_session_register(struct ksmbd_conn *conn,
@@ -198,29 +197,54 @@ int ksmbd_session_register(struct ksmbd_conn *conn,
198197
return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL));
199198
}
200199

201-
static void ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess)
200+
static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess)
202201
{
203202
struct channel *chann;
204203

205204
chann = xa_erase(&sess->ksmbd_chann_list, (long)conn);
206205
if (!chann)
207-
return;
206+
return -ENOENT;
208207

209208
kfree(chann);
209+
return 0;
210210
}
211211

212212
void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
213213
{
214214
struct ksmbd_session *sess;
215215
unsigned long id;
216216

217+
down_write(&sessions_table_lock);
218+
if (conn->binding) {
219+
int bkt;
220+
struct hlist_node *tmp;
221+
222+
hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
223+
if (!ksmbd_chann_del(conn, sess) &&
224+
xa_empty(&sess->ksmbd_chann_list)) {
225+
hash_del(&sess->hlist);
226+
ksmbd_session_destroy(sess);
227+
}
228+
}
229+
}
230+
217231
xa_for_each(&conn->sessions, id, sess) {
232+
unsigned long chann_id;
233+
struct channel *chann;
234+
235+
xa_for_each(&sess->ksmbd_chann_list, chann_id, chann) {
236+
if (chann->conn != conn)
237+
ksmbd_conn_set_exiting(chann->conn);
238+
}
239+
218240
ksmbd_chann_del(conn, sess);
219241
if (xa_empty(&sess->ksmbd_chann_list)) {
220242
xa_erase(&conn->sessions, sess->id);
243+
hash_del(&sess->hlist);
221244
ksmbd_session_destroy(sess);
222245
}
223246
}
247+
up_write(&sessions_table_lock);
224248
}
225249

226250
struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,

fs/ksmbd/smb2pdu.c

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2081,21 +2081,22 @@ int smb2_session_logoff(struct ksmbd_work *work)
20812081
struct smb2_logoff_rsp *rsp = smb2_get_msg(work->response_buf);
20822082
struct ksmbd_session *sess;
20832083
struct smb2_logoff_req *req = smb2_get_msg(work->request_buf);
2084+
u64 sess_id = le64_to_cpu(req->hdr.SessionId);
20842085

20852086
rsp->StructureSize = cpu_to_le16(4);
20862087
inc_rfc1001_len(work->response_buf, 4);
20872088

20882089
ksmbd_debug(SMB, "request\n");
20892090

2090-
ksmbd_conn_set_need_reconnect(conn);
2091+
ksmbd_all_conn_set_status(sess_id, KSMBD_SESS_NEED_RECONNECT);
20912092
ksmbd_close_session_fds(work);
2092-
ksmbd_conn_wait_idle(conn);
2093+
ksmbd_conn_wait_idle(conn, sess_id);
20932094

20942095
/*
20952096
* Re-lookup session to validate if session is deleted
20962097
* while waiting request complete
20972098
*/
2098-
sess = ksmbd_session_lookup(conn, le64_to_cpu(req->hdr.SessionId));
2099+
sess = ksmbd_session_lookup_all(conn, sess_id);
20992100
if (ksmbd_tree_conn_session_logoff(sess)) {
21002101
ksmbd_debug(SMB, "Invalid tid %d\n", req->hdr.Id.SyncId.TreeId);
21012102
rsp->hdr.Status = STATUS_NETWORK_NAME_DELETED;
@@ -2108,7 +2109,7 @@ int smb2_session_logoff(struct ksmbd_work *work)
21082109

21092110
ksmbd_free_user(sess->user);
21102111
sess->user = NULL;
2111-
ksmbd_conn_set_need_negotiate(conn);
2112+
ksmbd_all_conn_set_status(sess_id, KSMBD_SESS_NEED_NEGOTIATE);
21122113
return 0;
21132114
}
21142115

@@ -6863,7 +6864,7 @@ int smb2_lock(struct ksmbd_work *work)
68636864

68646865
nolock = 1;
68656866
/* check locks in connection list */
6866-
read_lock(&conn_list_lock);
6867+
down_read(&conn_list_lock);
68676868
list_for_each_entry(conn, &conn_list, conns_list) {
68686869
spin_lock(&conn->llist_lock);
68696870
list_for_each_entry_safe(cmp_lock, tmp2, &conn->lock_list, clist) {
@@ -6880,7 +6881,7 @@ int smb2_lock(struct ksmbd_work *work)
68806881
list_del(&cmp_lock->flist);
68816882
list_del(&cmp_lock->clist);
68826883
spin_unlock(&conn->llist_lock);
6883-
read_unlock(&conn_list_lock);
6884+
up_read(&conn_list_lock);
68846885

68856886
locks_free_lock(cmp_lock->fl);
68866887
kfree(cmp_lock);
@@ -6902,7 +6903,7 @@ int smb2_lock(struct ksmbd_work *work)
69026903
cmp_lock->start > smb_lock->start &&
69036904
cmp_lock->start < smb_lock->end) {
69046905
spin_unlock(&conn->llist_lock);
6905-
read_unlock(&conn_list_lock);
6906+
up_read(&conn_list_lock);
69066907
pr_err("previous lock conflict with zero byte lock range\n");
69076908
goto out;
69086909
}
@@ -6911,7 +6912,7 @@ int smb2_lock(struct ksmbd_work *work)
69116912
smb_lock->start > cmp_lock->start &&
69126913
smb_lock->start < cmp_lock->end) {
69136914
spin_unlock(&conn->llist_lock);
6914-
read_unlock(&conn_list_lock);
6915+
up_read(&conn_list_lock);
69156916
pr_err("current lock conflict with zero byte lock range\n");
69166917
goto out;
69176918
}
@@ -6922,14 +6923,14 @@ int smb2_lock(struct ksmbd_work *work)
69226923
cmp_lock->end >= smb_lock->end)) &&
69236924
!cmp_lock->zero_len && !smb_lock->zero_len) {
69246925
spin_unlock(&conn->llist_lock);
6925-
read_unlock(&conn_list_lock);
6926+
up_read(&conn_list_lock);
69266927
pr_err("Not allow lock operation on exclusive lock range\n");
69276928
goto out;
69286929
}
69296930
}
69306931
spin_unlock(&conn->llist_lock);
69316932
}
6932-
read_unlock(&conn_list_lock);
6933+
up_read(&conn_list_lock);
69336934
out_check_cl:
69346935
if (smb_lock->fl->fl_type == F_UNLCK && nolock) {
69356936
pr_err("Try to unlock nolocked range\n");

0 commit comments

Comments
 (0)