ksmbd: fix user-after-free from session log off

There is racy issue between smb2 session log off and smb2 session setup.
It will cause user-after-free from session log off.
This add session_lock when setting SMB2_SESSION_EXPIRED and referece
count to session struct not to free session while it is being used.

Cc: stable@vger.kernel.org # v5.15+
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-25282
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
This commit is contained in:
Namjae Jeon 2024-10-08 22:42:57 +09:00 committed by Steve French
parent 8cf0b93919
commit 7aa8804c0b
4 changed files with 34 additions and 6 deletions

View File

@ -177,9 +177,10 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn)
down_write(&conn->session_lock);
xa_for_each(&conn->sessions, id, sess) {
if (sess->state != SMB2_SESSION_VALID ||
time_after(jiffies,
sess->last_active + SMB2_SESSION_TIMEOUT)) {
if (atomic_read(&sess->refcnt) == 0 &&
(sess->state != SMB2_SESSION_VALID ||
time_after(jiffies,
sess->last_active + SMB2_SESSION_TIMEOUT))) {
xa_erase(&conn->sessions, sess->id);
hash_del(&sess->hlist);
ksmbd_session_destroy(sess);
@ -269,8 +270,6 @@ struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id)
down_read(&sessions_table_lock);
sess = __session_lookup(id);
if (sess)
sess->last_active = jiffies;
up_read(&sessions_table_lock);
return sess;
@ -289,6 +288,22 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
return sess;
}
void ksmbd_user_session_get(struct ksmbd_session *sess)
{
atomic_inc(&sess->refcnt);
}
void ksmbd_user_session_put(struct ksmbd_session *sess)
{
if (!sess)
return;
if (atomic_read(&sess->refcnt) <= 0)
WARN_ON(1);
else
atomic_dec(&sess->refcnt);
}
struct preauth_session *ksmbd_preauth_session_alloc(struct ksmbd_conn *conn,
u64 sess_id)
{
@ -393,6 +408,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);
ret = __init_smb2_session(sess);
if (ret)

View File

@ -61,6 +61,8 @@ struct ksmbd_session {
struct ksmbd_file_table file_table;
unsigned long last_active;
rwlock_t tree_conns_lock;
atomic_t refcnt;
};
static inline int test_session_flag(struct ksmbd_session *sess, int bit)
@ -104,4 +106,6 @@ void ksmbd_release_tree_conn_id(struct ksmbd_session *sess, int id);
int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name);
void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id);
int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id);
void ksmbd_user_session_get(struct ksmbd_session *sess);
void ksmbd_user_session_put(struct ksmbd_session *sess);
#endif /* __USER_SESSION_MANAGEMENT_H__ */

View File

@ -238,6 +238,8 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
} while (is_chained == true);
send:
if (work->sess)
ksmbd_user_session_put(work->sess);
if (work->tcon)
ksmbd_tree_connect_put(work->tcon);
smb3_preauth_hash_rsp(work);

View File

@ -605,8 +605,10 @@ int smb2_check_user_session(struct ksmbd_work *work)
/* Check for validity of user session */
work->sess = ksmbd_session_lookup_all(conn, sess_id);
if (work->sess)
if (work->sess) {
ksmbd_user_session_get(work->sess);
return 1;
}
ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
return -ENOENT;
}
@ -1740,6 +1742,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
}
conn->binding = true;
ksmbd_user_session_get(sess);
} else if ((conn->dialect < SMB30_PROT_ID ||
server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL) &&
(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
@ -1766,6 +1769,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
}
conn->binding = false;
ksmbd_user_session_get(sess);
}
work->sess = sess;
@ -2228,7 +2232,9 @@ int smb2_session_logoff(struct ksmbd_work *work)
}
ksmbd_destroy_file_table(&sess->file_table);
down_write(&conn->session_lock);
sess->state = SMB2_SESSION_EXPIRED;
up_write(&conn->session_lock);
ksmbd_free_user(sess->user);
sess->user = NULL;