APPLIED: [SRU][F][PATCH 0/1] CVE-2022-48733

[Roxana Nicolescu]

On 30/09/2024 05:06, Koichiro Den wrote:
> [Impact]
>
> btrfs: fix use-after-free after failure to create a snapshot
>
> 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.
>
> [Backport]
>
> Adjusted context due to missing commits:
> - 9babda9f33fd ("btrfs: Remove async_transid from btrfs_mksubvol/create_subvol/create_snapshot")
> - d0c2f4fa555e ("btrfs: make concurrent fsyncs wait less when waiting for a transaction commit")
>
> [Fix]
>
> Noble:  not affected
> Jammy:  fixed via stable
> Focal:  Backport - adjusted context, see [Backport]
> Bionic: fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty: won't fix
>
> [Test Case]
>
> Compile and boot tested (amd64 only).
>
> Ran Smatch and verified that with this backport, the following warn
> message disappeared:
>    fs/btrfs/ioctl.c:908 create_snapshot() warn: '&pending_snapshot->list' not removed from list
>
> Also, did snapshot testing and verified that btrfs_commit_transaction()
> triggered by it succeeded without any issues (amd64 only).
>
> [Where problems could occur]
>
> This fix affects those who use btrfs snapshot feature, an issue with
> this fix would be visible to the user via unpredicted system behaviour
> or a system crash induced by use-after-free.
>
>
> Filipe Manana (1):
>    btrfs: fix use-after-free after failure to create a snapshot
>
>   fs/btrfs/ioctl.c       |  5 +----
>   fs/btrfs/transaction.c | 24 ++++++++++++++++++++++++
>   fs/btrfs/transaction.h |  2 ++
>   3 files changed, 27 insertions(+), 4 deletions(-)
>
Applied to focal:linux master-next branch. Thanks!