Commit fa4cdb8c authored by Namjae Jeon's avatar Namjae Jeon Committed by Steve French
Browse files

ksmbd: fix session use-after-free in multichannel connection 



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: default avatarSean Heelan <seanheelan@gmail.com>
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent f64a72bc

fs/smb/server/auth.c

+2 −2
Original line number Diff line number Diff line
@@ -1016,9 +1016,9 @@ static int ksmbd_get_encryption_key(struct ksmbd_work *work, __u64 ses_id, 


	ses_enc_key = enc ? sess->smb3encryptionkey :

		sess->smb3decryptionkey;

	if (enc)

		ksmbd_user_session_get(sess);

	memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);

	if (!enc)

		ksmbd_user_session_put(sess);



	return 0;

}

fs/smb/server/mgmt/user_session.c

+8 −6
Original line number Diff line number Diff line
@@ -181,7 +181,7 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn) 
	down_write(&sessions_table_lock);

	down_write(&conn->session_lock);

	xa_for_each(&conn->sessions, id, sess) {

		if (atomic_read(&sess->refcnt) == 0 &&

		if (atomic_read(&sess->refcnt) <= 1 &&

		    (sess->state != SMB2_SESSION_VALID ||

		     time_after(jiffies,

			       sess->last_active + SMB2_SESSION_TIMEOUT))) {
@@ -233,6 +233,7 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn) 
				down_write(&conn->session_lock);

				xa_erase(&conn->sessions, sess->id);

				up_write(&conn->session_lock);

				if (atomic_dec_and_test(&sess->refcnt))

					ksmbd_session_destroy(sess);

			}

		}
@@ -252,6 +253,7 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn) 
		if (xa_empty(&sess->ksmbd_chann_list)) {

			xa_erase(&conn->sessions, sess->id);

			hash_del(&sess->hlist);

			if (atomic_dec_and_test(&sess->refcnt))

				ksmbd_session_destroy(sess);

		}

	}
@@ -328,8 +330,8 @@ void ksmbd_user_session_put(struct ksmbd_session *sess) 


	if (atomic_read(&sess->refcnt) <= 0)

		WARN_ON(1);

	else

		atomic_dec(&sess->refcnt);

	else if (atomic_dec_and_test(&sess->refcnt))

		ksmbd_session_destroy(sess);

}



struct preauth_session *ksmbd_preauth_session_alloc(struct ksmbd_conn *conn,
@@ -436,7 +438,7 @@ static struct ksmbd_session *__session_create(int protocol) 
	xa_init(&sess->rpc_handle_list);

	sess->sequence_number = 1;

	rwlock_init(&sess->tree_conns_lock);

	atomic_set(&sess->refcnt, 1);

	atomic_set(&sess->refcnt, 2);



	ret = __init_smb2_session(sess);

	if (ret)

fs/smb/server/smb2pdu.c

+4 −3
Original line number Diff line number Diff line
@@ -2235,13 +2235,14 @@ int smb2_session_logoff(struct ksmbd_work *work) 
		return -ENOENT;

	}



	ksmbd_destroy_file_table(&sess->file_table);

	down_write(&conn->session_lock);

	sess->state = SMB2_SESSION_EXPIRED;

	up_write(&conn->session_lock);



	if (sess->user) {

		ksmbd_free_user(sess->user);

		sess->user = NULL;

	}

	ksmbd_all_conn_set_status(sess_id, KSMBD_SESS_NEED_NEGOTIATE);



	rsp->StructureSize = cpu_to_le16(4);