btrfs: fix match incorrectly in dev_args_match_device

[PATCH] btrfs: fix match incorrectly in dev_args_match_device

Posted by Liu Shixin 3 years, 5 months ago

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: syzbot+031687116258450f9853@syzkaller.appspotmail.com
Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 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

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

Posted by David Sterba 3 years, 5 months ago

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: syzbot+031687116258450f9853@syzkaller.appspotmail.com
> Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Added to misc-next, thanks.

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

Posted by Nikolay Borisov 3 years, 5 months ago


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: syzbot+031687116258450f9853@syzkaller.appspotmail.com
> Fixes: 562d7b1512f7 ("btrfs: handle device lookup with btrfs_dev_lookup_args")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>   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;
>   }
>   
>   /*