• Filipe Manana's avatar
    btrfs: fix use-after-free after failure to create a snapshot · 28b21c55
    Filipe Manana authored
    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: default avatarFilipe Manana <fdmanana@suse.com>
    Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
    28b21c55
transaction.h 7.65 KB