Commit 29da3310 authored Jul 05, 2024 by Filipe Manana Committed by Baokun Li Jul 05, 2024

btrfs: fix use-after-free after failure to create a snapshot

mainline inclusion
from mainline-v5.17-rc3
commit 28b21c55
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IA72FO
CVE: CVE-2022-48733

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28b21c558a3753171097193b6f6602a94169093a



--------------------------------

At ioctl.c:create_snapshot(), we allocate a pending snapshot structure and
then attach it to the transaction's list of pending snapshots. After that
we call btrfs_commit_transaction(), and if that returns an error we jump
to 'fail' label, where we kfree() the pending snapshot structure. This can
result in a later use-after-free of the pending snapshot:

1) We allocated the pending snapshot and added it to the transaction's
   list of pending snapshots;

2) We call btrfs_commit_transaction(), and it fails either at the first
   call to btrfs_run_delayed_refs() or btrfs_start_dirty_block_groups().
   In both cases, we don't abort the transaction and we release our
   transaction handle. We jump to the 'fail' label and free the pending
   snapshot structure. We return with the pending snapshot still in the
   transaction's list;

3) Another task commits the transaction. This time there's no error at
   all, and then during the transaction commit it accesses a pointer
   to the pending snapshot structure that the snapshot creation task
   has already freed, resulting in a user-after-free.

This issue could actually be detected by smatch, which produced the
following warning:

  fs/btrfs/ioctl.c:843 create_snapshot() warn: '&pending_snapshot->list' not removed from list

So fix this by not having the snapshot creation ioctl directly add the
pending snapshot to the transaction's list. Instead add the pending
snapshot to the transaction handle, and then at btrfs_commit_transaction()
we add the snapshot to the list only when we can guarantee that any error
returned after that point will result in a transaction abort, in which
case the ioctl code can safely free the pending snapshot and no one can
access it anymore.

CC: stable@vger.kernel.org # 5.10+
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

Conflicts:
	fs/btrfs/ioctl.c
	fs/btrfs/transaction.c
[Because commit 74e97958121a ("btrfs: qgroup: fix qgroup prealloc rsv leak
in subvolume operations") was merged before this commit and current commit
d0c2f4fa555eh ("btrfs: make concurrent fsyncs wait less when waiting for a
transaction commit") has not been merged yet.]
Signed-off-by: Baokun Li <libaokun1@huawei.com>

parent 3f2841fc

fs/btrfs/ioctl.c

+1 −4

Original line number	Diff line number	Diff line
		@@ -868,10 +868,7 @@ static int create_snapshot(struct btrfs_root root, struct inode dir,
		btrfs_qgroup_convert_reserved_meta(root, qgroup_reserved);
		qgroup_reserved = 0;

		spin_lock(&fs_info->trans_lock);
		list_add(&pending_snapshot->list,
		&trans->transaction->pending_snapshots);
		spin_unlock(&fs_info->trans_lock);
		trans->pending_snapshot = pending_snapshot;

		ret = btrfs_commit_transaction(trans);
		if (ret)

fs/btrfs/transaction.c

+24 −0

Original line number	Diff line number	Diff line
		@@ -2038,6 +2038,27 @@ static inline void btrfs_wait_delalloc_flush(struct btrfs_fs_info *fs_info)
		btrfs_wait_ordered_roots(fs_info, U64_MAX, 0, (u64)-1);
		}

		/*
		* Add a pending snapshot associated with the given transaction handle to the
		* respective handle. This must be called after the transaction commit started
		* and while holding fs_info->trans_lock.
		* This serves to guarantee a caller of btrfs_commit_transaction() that it can
		* safely free the pending snapshot pointer in case btrfs_commit_transaction()
		* returns an error.
		*/
		static void add_pending_snapshot(struct btrfs_trans_handle *trans)
		{
		struct btrfs_transaction *cur_trans = trans->transaction;

		if (!trans->pending_snapshot)
		return;

		lockdep_assert_held(&trans->fs_info->trans_lock);
		ASSERT(cur_trans->state >= TRANS_STATE_COMMIT_START);

		list_add(&trans->pending_snapshot->list, &cur_trans->pending_snapshots);
		}

		int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
		{
		struct btrfs_fs_info *fs_info = trans->fs_info;
		@@ -2124,6 +2145,8 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)

		spin_lock(&fs_info->trans_lock);
		if (cur_trans->state >= TRANS_STATE_COMMIT_START) {
		add_pending_snapshot(trans);

		spin_unlock(&fs_info->trans_lock);
		refcount_inc(&cur_trans->use_count);
		ret = btrfs_end_transaction(trans);
		@@ -2206,6 +2229,7 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
		* COMMIT_DOING so make sure to wait for num_writers to == 1 again.
		*/
		spin_lock(&fs_info->trans_lock);
		add_pending_snapshot(trans);
		cur_trans->state = TRANS_STATE_COMMIT_DOING;
		spin_unlock(&fs_info->trans_lock);
		wait_event(cur_trans->writer_wait,

fs/btrfs/transaction.h

+2 −0

Original line number	Diff line number	Diff line
		@@ -122,6 +122,8 @@ struct btrfs_trans_handle {
		struct btrfs_transaction *transaction;
		struct btrfs_block_rsv *block_rsv;
		struct btrfs_block_rsv *orig_rsv;
		/* Set by a task that wants to create a snapshot. */
		struct btrfs_pending_snapshot *pending_snapshot;
		refcount_t use_count;
		unsigned int type;
		/*