Re: [Internal Review] [Patch] btrfs: fix use-after-free of block device file in __btrfs_free_extra_devids()

["Daniel Walker (danielwa)" <danielwa@cisco.com>]

Looks fine for release to me.


On Thu, Jan 23, 2025 at 03:41:41AM -0800, Shubham Pushpkar wrote:
> From: Zhihao Cheng <chengzhihao1@huawei.com>
> 
> commit aec8e6bf839101784f3ef037dcdb9432c3f32343 ("btrfs:
> fix use-after-free of block device file in __btrfs_free_extra_devids()")
> 
> Mounting btrfs from two images (which have the same one fsid and two
> different dev_uuids) in certain executing order may trigger an UAF for
> variable 'device->bdev_file' in __btrfs_free_extra_devids(). And
> following are the details:
> 
> 1. Attach image_1 to loop0, attach image_2 to loop1, and scan btrfs
>    devices by ioctl(BTRFS_IOC_SCAN_DEV):
> 
>              /  btrfs_device_1 → loop0
>    fs_device
>              \  btrfs_device_2 → loop1
> 2. mount /dev/loop0 /mnt
>    btrfs_open_devices
>     btrfs_device_1->bdev_file = btrfs_get_bdev_and_sb(loop0)
>     btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>    btrfs_fill_super
>     open_ctree
>      fail: btrfs_close_devices // -ENOMEM
> 	    btrfs_close_bdev(btrfs_device_1)
>              fput(btrfs_device_1->bdev_file)
> 	      // btrfs_device_1->bdev_file is freed
> 	    btrfs_close_bdev(btrfs_device_2)
>              fput(btrfs_device_2->bdev_file)
> 
> 3. mount /dev/loop1 /mnt
>    btrfs_open_devices
>     btrfs_get_bdev_and_sb(&bdev_file)
>      // EIO, btrfs_device_1->bdev_file is not assigned,
>      // which points to a freed memory area
>     btrfs_device_2->bdev_file = btrfs_get_bdev_and_sb(loop1)
>    btrfs_fill_super
>     open_ctree
>      btrfs_free_extra_devids
>       if (btrfs_device_1->bdev_file)
>        fput(btrfs_device_1->bdev_file) // UAF !
> 
> Fix it by setting 'device->bdev_file' as 'NULL' after closing the
> btrfs_device in btrfs_close_one_device().
> 
> Fixes: CVE-2024-50217
> Fixes: 142388194191 ("btrfs: do not background blkdev_put()")
> CC: stable@vger.kernel.org # 4.19+
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=219408
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> Signed-off-by: David Sterba <dsterba@suse.com>
> (cherry picked from commit aec8e6bf839101784f3ef037dcdb9432c3f32343)
> Signed-off-by: Shubham Pushpkar <spushpka@cisco.com>
> ---
>  fs/btrfs/volumes.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index b9a0b26d08e1..ab2412542ce5 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1176,6 +1176,7 @@ static void btrfs_close_one_device(struct btrfs_device *device)
>  	if (device->bdev) {
>  		fs_devices->open_devices--;
>  		device->bdev = NULL;
> +		device->bdev_file = NULL;
>  	}
>  	clear_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
>  	btrfs_destroy_dev_zone_info(device);
> -- 
> 2.35.6
>