Skip to content

Commit 3980770

Browse files
namjaejeongregkh
authored andcommitted
ksmbd: fix session use-after-free in multichannel connection
commit fa4cdb8 upstream. There is a race condition between session setup and ksmbd_sessions_deregister. The session can be freed before the connection is added to channel list of session. This patch check reference count of session before freeing it. Cc: stable@vger.kernel.org Reported-by: Sean Heelan <seanheelan@gmail.com> Signed-off-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 a8a8ae3 commit 3980770

3 files changed

Lines changed: 14 additions & 11 deletions

File tree

fs/smb/server/auth.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,9 +1012,9 @@ static int ksmbd_get_encryption_key(struct ksmbd_work *work, __u64 ses_id,
10121012

10131013
ses_enc_key = enc ? sess->smb3encryptionkey :
10141014
sess->smb3decryptionkey;
1015-
if (enc)
1016-
ksmbd_user_session_get(sess);
10171015
memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);
1016+
if (!enc)
1017+
ksmbd_user_session_put(sess);
10181018

10191019
return 0;
10201020
}

fs/smb/server/mgmt/user_session.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn)
180180
down_write(&sessions_table_lock);
181181
down_write(&conn->session_lock);
182182
xa_for_each(&conn->sessions, id, sess) {
183-
if (atomic_read(&sess->refcnt) == 0 &&
183+
if (atomic_read(&sess->refcnt) <= 1 &&
184184
(sess->state != SMB2_SESSION_VALID ||
185185
time_after(jiffies,
186186
sess->last_active + SMB2_SESSION_TIMEOUT))) {
@@ -232,7 +232,8 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
232232
down_write(&conn->session_lock);
233233
xa_erase(&conn->sessions, sess->id);
234234
up_write(&conn->session_lock);
235-
ksmbd_session_destroy(sess);
235+
if (atomic_dec_and_test(&sess->refcnt))
236+
ksmbd_session_destroy(sess);
236237
}
237238
}
238239
}
@@ -251,7 +252,8 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
251252
if (xa_empty(&sess->ksmbd_chann_list)) {
252253
xa_erase(&conn->sessions, sess->id);
253254
hash_del(&sess->hlist);
254-
ksmbd_session_destroy(sess);
255+
if (atomic_dec_and_test(&sess->refcnt))
256+
ksmbd_session_destroy(sess);
255257
}
256258
}
257259
up_write(&conn->session_lock);
@@ -327,8 +329,8 @@ void ksmbd_user_session_put(struct ksmbd_session *sess)
327329

328330
if (atomic_read(&sess->refcnt) <= 0)
329331
WARN_ON(1);
330-
else
331-
atomic_dec(&sess->refcnt);
332+
else if (atomic_dec_and_test(&sess->refcnt))
333+
ksmbd_session_destroy(sess);
332334
}
333335

334336
struct preauth_session *ksmbd_preauth_session_alloc(struct ksmbd_conn *conn,
@@ -433,7 +435,7 @@ static struct ksmbd_session *__session_create(int protocol)
433435
xa_init(&sess->rpc_handle_list);
434436
sess->sequence_number = 1;
435437
rwlock_init(&sess->tree_conns_lock);
436-
atomic_set(&sess->refcnt, 1);
438+
atomic_set(&sess->refcnt, 2);
437439

438440
ret = __init_smb2_session(sess);
439441
if (ret)

fs/smb/server/smb2pdu.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2231,13 +2231,14 @@ int smb2_session_logoff(struct ksmbd_work *work)
22312231
return -ENOENT;
22322232
}
22332233

2234-
ksmbd_destroy_file_table(&sess->file_table);
22352234
down_write(&conn->session_lock);
22362235
sess->state = SMB2_SESSION_EXPIRED;
22372236
up_write(&conn->session_lock);
22382237

2239-
ksmbd_free_user(sess->user);
2240-
sess->user = NULL;
2238+
if (sess->user) {
2239+
ksmbd_free_user(sess->user);
2240+
sess->user = NULL;
2241+
}
22412242
ksmbd_all_conn_set_status(sess_id, KSMBD_SESS_NEED_NEGOTIATE);
22422243

22432244
rsp->StructureSize = cpu_to_le16(4);

0 commit comments

Comments
 (0)