btrfs: fix match incorrectly in dev_args_match_device

Message ID 20221103083301.626561-1-liushixin2@huawei.com
State New
Headers
Series btrfs: fix match incorrectly in dev_args_match_device |

Commit Message

Liu Shixin Nov. 3, 2022, 8:33 a.m. UTC
  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(-)
  

Comments

Nikolay Borisov Nov. 3, 2022, 8:20 a.m. UTC | #1
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;
>   }
>   
>   /*
  
David Sterba Nov. 3, 2022, 4:33 p.m. UTC | #2
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.
  

Patch

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