Re: [PATCH] btrfs: check for missing device in btrfs_trim_fs

[Filipe Manana <fdmanana@gmail.com>]

On Sun, Jul 4, 2021 at 12:17 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> A fstrim on a degraded raid1 can trigger the following null pointer
> dereference:
>
> BTRFS info (device loop0): allowing degraded mounts
> BTRFS info (device loop0): disk space caching is enabled
> BTRFS info (device loop0): has skinny extents
> BTRFS warning (device loop0): devid 2 uuid 97ac16f7-e14d-4db1-95bc-3d489b424adb is missing
> BTRFS warning (device loop0): devid 2 uuid 97ac16f7-e14d-4db1-95bc-3d489b424adb is missing
> BTRFS info (device loop0): enabling ssd optimizations
> BUG: kernel NULL pointer dereference, address: 0000000000000620
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP NOPTI
> CPU: 0 PID: 4574 Comm: fstrim Not tainted 5.13.0-rc7+ #31
> Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> RIP: 0010:btrfs_trim_fs+0x199/0x4a0 [btrfs]
> Code: 24 10 65 4c 8b 34 25 00 70 01 00 48 c7 44 24 38 00 00 10 00 48 8b 45 50 48 c7 44 24 40 00 00 00 00 48 c7 44 24 30 00 00 00 00 <48> 8b 80 20 06 00 00 48 8b 80 90 00 00 00 48 8b 40 68 f6 c4 01 0f
> RSP: 0018:ffff959541797d28 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: ffff946f84eca508 RCX: a7a67937adff8608
> RDX: ffff946e8122d000 RSI: 0000000000000000 RDI: ffffffffc02fdbf0
> RBP: ffff946ea4615000 R08: 0000000000000001 R09: 0000000000000000
> R10: 0000000000000000 R11: ffff946e8122d960 R12: 0000000000000000
> R13: ffff959541797db8 R14: ffff946e8122d000 R15: ffff959541797db8
> FS:  00007f55917a5080(0000) GS:ffff946f9bc00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000620 CR3: 000000002d2c8001 CR4: 00000000000706f0
> Call Trace:
> btrfs_ioctl_fitrim+0x167/0x260 [btrfs]
> btrfs_ioctl+0x1c00/0x2fe0 [btrfs]
> ? selinux_file_ioctl+0x140/0x240
> ? syscall_trace_enter.constprop.0+0x188/0x240
> ? __x64_sys_ioctl+0x83/0xb0
> __x64_sys_ioctl+0x83/0xb0
> do_syscall_64+0x40/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Reproducer:
>
>   Create raid1 btrfs:
>         mkfs.btrfs -fq -draid1 -mraid1 /dev/loop0 /dev/loop1
>         mount /dev/loop0 /btrfs
>
>   Create some data:
>         _sysbench prepare /btrfs 10 2g 6

This step isn't needed, it's confusing to suggest the filesystem needs
to have some data in order to trigger the bug.

>
>   Mount with one the device missing:
>         umount /btrfs
>         btrfs dev scan --forget
>         mount -o degraded /dev/loop0 /btrfs
>
>   Run fstrim:
>         fstrim /btrfs
>
> The reason is we call btrfs_trim_free_extents() for the missing device,
> which uses device->bdev (NULL for missing device) to find if the device
> supports discard.
>
> Fix is to check if the device is missing before calling
> btrfs_trim_free_extents().
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Other than that, it looks good, thanks.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

> ---
>  fs/btrfs/extent-tree.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index d296483d148f..268ce58d4569 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -6019,6 +6019,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range)
>         mutex_lock(&fs_info->fs_devices->device_list_mutex);
>         devices = &fs_info->fs_devices->devices;
>         list_for_each_entry(device, devices, dev_list) {
> +               if (test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state))
> +                       continue;
> +
>                 ret = btrfs_trim_free_extents(device, &group_trimmed);
>                 if (ret) {
>                         dev_failed++;
> --
> 2.31.1
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”