Commit 48623ec3 authored by Werner Kasselman's avatar Werner Kasselman Committed by Steve French
Browse files

ksmbd: fix use-after-free and NULL deref in smb_grant_oplock() 



smb_grant_oplock() has two issues in the oplock publication sequence:

1) opinfo is linked into ci->m_op_list (via opinfo_add) before
   add_lease_global_list() is called.  If add_lease_global_list()
   fails (kmalloc returns NULL), the error path frees the opinfo
   via __free_opinfo() while it is still linked in ci->m_op_list.
   Concurrent m_op_list readers (opinfo_get_list, or direct iteration
   in smb_break_all_levII_oplock) dereference the freed node.

2) opinfo->o_fp is assigned after add_lease_global_list() publishes
   the opinfo on the global lease list.  A concurrent
   find_same_lease_key() can walk the lease list and dereference
   opinfo->o_fp->f_ci while o_fp is still NULL.

Fix by restructuring the publication sequence to eliminate post-publish
failure:

- Set opinfo->o_fp before any list publication (fixes NULL deref).
- Preallocate lease_table via alloc_lease_table() before opinfo_add()
  so add_lease_global_list() becomes infallible after publication.
- Keep the original m_op_list publication order (opinfo_add before
  lease list) so concurrent opens via same_client_has_lease() and
  opinfo_get_list() still see the in-flight grant.
- Use opinfo_put() instead of __free_opinfo() on err_out so that
  the RCU-deferred free path is used.

This also requires splitting add_lease_global_list() to take a
preallocated lease_table and changing its return type from int to void,
since it can no longer fail.

Fixes: 1dfd062c ("ksmbd: fix use-after-free by using call_rcu() for oplock_info")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarWerner Kasselman <werner@verivus.com>
Reviewed-by: default avatarChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 9bbb19d2

fs/smb/server/oplock.c

+45 −27
Original line number Diff line number Diff line
@@ -82,11 +82,19 @@ static void lease_del_list(struct oplock_info *opinfo) 
	spin_unlock(&lb->lb_lock);

}



static void lb_add(struct lease_table *lb)

static struct lease_table *alloc_lease_table(struct oplock_info *opinfo)

{

	write_lock(&lease_list_lock);

	list_add(&lb->l_entry, &lease_table_list);

	write_unlock(&lease_list_lock);

	struct lease_table *lb;



	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);

	if (!lb)

		return NULL;



	memcpy(lb->client_guid, opinfo->conn->ClientGUID,

	       SMB2_CLIENT_GUID_SIZE);

	INIT_LIST_HEAD(&lb->lease_list);

	spin_lock_init(&lb->lb_lock);

	return lb;

}



static int alloc_lease(struct oplock_info *opinfo, struct lease_ctx_info *lctx)
@@ -1042,34 +1050,27 @@ static void copy_lease(struct oplock_info *op1, struct oplock_info *op2) 
	lease2->version = lease1->version;

}



static int add_lease_global_list(struct oplock_info *opinfo)

static void add_lease_global_list(struct oplock_info *opinfo,

				  struct lease_table *new_lb)

{

	struct lease_table *lb;



	read_lock(&lease_list_lock);

	write_lock(&lease_list_lock);

	list_for_each_entry(lb, &lease_table_list, l_entry) {

		if (!memcmp(lb->client_guid, opinfo->conn->ClientGUID,

			    SMB2_CLIENT_GUID_SIZE)) {

			opinfo->o_lease->l_lb = lb;

			lease_add_list(opinfo);

			read_unlock(&lease_list_lock);

			return 0;

			write_unlock(&lease_list_lock);

			kfree(new_lb);

			return;

		}

	}

	read_unlock(&lease_list_lock);



	lb = kmalloc_obj(struct lease_table, KSMBD_DEFAULT_GFP);

	if (!lb)

		return -ENOMEM;



	memcpy(lb->client_guid, opinfo->conn->ClientGUID,

	       SMB2_CLIENT_GUID_SIZE);

	INIT_LIST_HEAD(&lb->lease_list);

	spin_lock_init(&lb->lb_lock);

	opinfo->o_lease->l_lb = lb;

	opinfo->o_lease->l_lb = new_lb;

	lease_add_list(opinfo);

	lb_add(lb);

	return 0;

	list_add(&new_lb->l_entry, &lease_table_list);

	write_unlock(&lease_list_lock);

}



static void set_oplock_level(struct oplock_info *opinfo, int level,
@@ -1189,6 +1190,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid, 
	int err = 0;

	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;

	struct ksmbd_inode *ci = fp->f_ci;

	struct lease_table *new_lb = NULL;

	bool prev_op_has_lease;

	__le32 prev_op_state = 0;
@@ -1291,21 +1293,37 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid, 
	set_oplock_level(opinfo, req_op_level, lctx);



out:

	opinfo_count_inc(fp);

	opinfo_add(opinfo, fp);



	/*

	 * Set o_fp before any publication so that concurrent readers

	 * (e.g. find_same_lease_key() on the lease list) that

	 * dereference opinfo->o_fp don't hit a NULL pointer.

	 *

	 * Keep the original publication order so concurrent opens can

	 * still observe the in-flight grant via ci->m_op_list, but make

	 * everything after opinfo_add() no-fail by preallocating any new

	 * lease_table first.

	 */

	opinfo->o_fp = fp;

	if (opinfo->is_lease) {

		err = add_lease_global_list(opinfo);

		if (err)

		new_lb = alloc_lease_table(opinfo);

		if (!new_lb) {

			err = -ENOMEM;

			goto err_out;

		}

	}



	opinfo_count_inc(fp);

	opinfo_add(opinfo, fp);



	if (opinfo->is_lease)

		add_lease_global_list(opinfo, new_lb);



	rcu_assign_pointer(fp->f_opinfo, opinfo);

	opinfo->o_fp = fp;



	return 0;

err_out:

	__free_opinfo(opinfo);

	kfree(new_lb);

	opinfo_put(opinfo);

	return err;

}