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

ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency 



Under high concurrency, A tree-connection object (tcon) is freed on
a disconnect path while another path still holds a reference and later
executes *_put()/write on it.

Reported-by: default avatarQianchang Zhao <pioooooooooip@gmail.com>
Reported-by: default avatarZhitong Liu <liuzhitong1993@gmail.com>
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 3316a8fc

fs/smb/server/mgmt/tree_connect.c

+4 −14
Original line number Diff line number Diff line
@@ -78,7 +78,6 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name) 
	tree_conn->t_state = TREE_NEW;

	status.tree_conn = tree_conn;

	atomic_set(&tree_conn->refcount, 1);

	init_waitqueue_head(&tree_conn->refcount_q);



	ret = xa_err(xa_store(&sess->tree_conns, tree_conn->id, tree_conn,

			      KSMBD_DEFAULT_GFP));
@@ -100,14 +99,8 @@ ksmbd_tree_conn_connect(struct ksmbd_work *work, const char *share_name) 


void ksmbd_tree_connect_put(struct ksmbd_tree_connect *tcon)

{

	/*

	 * Checking waitqueue to releasing tree connect on

	 * tree disconnect. waitqueue_active is safe because it

	 * uses atomic operation for condition.

	 */

	if (!atomic_dec_return(&tcon->refcount) &&

	    waitqueue_active(&tcon->refcount_q))

		wake_up(&tcon->refcount_q);

	if (atomic_dec_and_test(&tcon->refcount))

		kfree(tcon);

}



int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
@@ -119,13 +112,10 @@ int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess, 
	xa_erase(&sess->tree_conns, tree_conn->id);

	write_unlock(&sess->tree_conns_lock);



	if (!atomic_dec_and_test(&tree_conn->refcount))

		wait_event(tree_conn->refcount_q,

			   atomic_read(&tree_conn->refcount) == 0);



	ret = ksmbd_ipc_tree_disconnect_request(sess->id, tree_conn->id);

	ksmbd_release_tree_conn_id(sess, tree_conn->id);

	ksmbd_share_config_put(tree_conn->share_conf);

	if (atomic_dec_and_test(&tree_conn->refcount))

		kfree(tree_conn);

	return ret;

}

fs/smb/server/mgmt/tree_connect.h

+0 −1
Original line number Diff line number Diff line
@@ -33,7 +33,6 @@ struct ksmbd_tree_connect { 
	int				maximal_access;

	bool				posix_extensions;

	atomic_t			refcount;

	wait_queue_head_t		refcount_q;

	unsigned int			t_state;

};

fs/smb/server/smb2pdu.c

+0 −3
Original line number Diff line number Diff line
@@ -2190,7 +2190,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work) 
		goto err_out;

	}



	WARN_ON_ONCE(atomic_dec_and_test(&tcon->refcount));

	tcon->t_state = TREE_DISCONNECTED;

	write_unlock(&sess->tree_conns_lock);
@@ -2200,8 +2199,6 @@ int smb2_tree_disconnect(struct ksmbd_work *work) 
		goto err_out;

	}



	work->tcon = NULL;



	rsp->StructureSize = cpu_to_le16(4);

	err = ksmbd_iov_pin_rsp(work, rsp,

				sizeof(struct smb2_tree_disconnect_rsp));