2022-11-03 08:09:45

by Liu Shixin

Subject: [PATCH] btrfs: fix match incorrectly in dev_args_match_device

syzkaller found an assert failed:

assertion failed: (args->devid != (u64)-1) || args->missing, in fs/btrfs/volumes.c:6921

This can be trigger when we set devid to (u64)-1) by ioctl. In this case,
the match of devid will be skipped and the match of device may be succeed
incorrectly.

Patch 562d7b1512f7 introduced this function which is used to match device.
This function contaions two matching scenarios, we can distinguish them by
checking the value of args->missing rather than check whether args->devid
and args->uuid is default value.

Reported-by: [email protected]
Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
Signed-off-by: Liu Shixin <[email protected]>
---
fs/btrfs/volumes.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 94ba46d57920..bf2d886cfb4b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6918,18 +6918,18 @@ static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args,
static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args,
const struct btrfs_device *device)
{
- ASSERT((args->devid != (u64)-1) || args->missing);
+ if (args->missing) {
+ if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state) &&
+ !device->bdev)
+ return true;
+ return false;
+ }

- if ((args->devid != (u64)-1) && device->devid != args->devid)
+ if (device->devid != args->devid)
return false;
if (args->uuid && memcmp(device->uuid, args->uuid, BTRFS_UUID_SIZE) != 0)
return false;
- if (!args->missing)
- return true;
- if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state) &&
- !device->bdev)
- return true;
- return false;
+ return true;
}

/*
--
2.25.1

2022-11-03 08:31:27

by Nikolay Borisov

Subject: Re: [PATCH] btrfs: fix match incorrectly in dev_args_match_device

On 3.11.22 г. 10:33 ч., Liu Shixin wrote:
> syzkaller found an assert failed:
>
> assertion failed: (args->devid != (u64)-1) || args->missing, in fs/btrfs/volumes.c:6921
>
> This can be trigger when we set devid to (u64)-1) by ioctl. In this case,
> the match of devid will be skipped and the match of device may be succeed
> incorrectly.
>
> Patch 562d7b1512f7 introduced this function which is used to match device.
> This function contaions two matching scenarios, we can distinguish them by
> checking the value of args->missing rather than check whether args->devid
> and args->uuid is default value.
>
> Reported-by: [email protected]
> Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
> Signed-off-by: Liu Shixin <[email protected]>

Reviewed-by: Nikolay Borisov <[email protected]>

> ---
> fs/btrfs/volumes.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 94ba46d57920..bf2d886cfb4b 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6918,18 +6918,18 @@ static bool dev_args_match_fs_devices(const struct btrfs_dev_lookup_args *args,
> static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args,
> const struct btrfs_device *device)
> {
> - ASSERT((args->devid != (u64)-1) || args->missing);
> + if (args->missing) {
> + if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state) &&
> + !device->bdev)
> + return true;
> + return false;
> + }
>
> - if ((args->devid != (u64)-1) && device->devid != args->devid)
> + if (device->devid != args->devid)
> return false;
> if (args->uuid && memcmp(device->uuid, args->uuid, BTRFS_UUID_SIZE) != 0)
> return false;
> - if (!args->missing)
> - return true;
> - if (test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state) &&
> - !device->bdev)
> - return true;
> - return false;
> + return true;
> }
>
> /*

2022-11-03 17:06:45

by David Sterba

Subject: Re: [PATCH] btrfs: fix match incorrectly in dev_args_match_device

On Thu, Nov 03, 2022 at 04:33:01PM +0800, Liu Shixin wrote:
> syzkaller found an assert failed:
>
> assertion failed: (args->devid != (u64)-1) || args->missing, in fs/btrfs/volumes.c:6921
>
> This can be trigger when we set devid to (u64)-1) by ioctl. In this case,
> the match of devid will be skipped and the match of device may be succeed
> incorrectly.
>
> Patch 562d7b1512f7 introduced this function which is used to match device.
> This function contaions two matching scenarios, we can distinguish them by
> checking the value of args->missing rather than check whether args->devid
> and args->uuid is default value.
>
> Reported-by: [email protected]
> Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
> Signed-off-by: Liu Shixin <[email protected]>

Added to misc-next, thanks.