| Message ID | 20210531092601.107452-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Promote debugging asserts to full-flegded checks in validate_super | expand |
On 31/05/2021 11:26, Nikolay Borisov wrote: > Syzbot managed to trigger this assert while performing its fuzzing. > Turns out it's better to have those asserts turned into full-fledged > checks so that in case buggy btrfs images are mounted the users gets > an error and mounting is stopped. Alternatively with CONFIG_BTRFS_ASSERT > disabled such image would have been erroneously allowed to be mounted. Agreed, Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On 2021/5/31 下午5:26, Nikolay Borisov wrote: > Syzbot managed to trigger this assert while performing its fuzzing. > Turns out it's better to have those asserts turned into full-fledged > checks so that in case buggy btrfs images are mounted the users gets > an error and mounting is stopped. Alternatively with CONFIG_BTRFS_ASSERT > disabled such image would have been erroneously allowed to be mounted. > > Reported-by: syzbot+a6bf271c02e4fe66b4e4@syzkaller.appspotmail.com > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/disk-io.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 99757939f8b0..cff694fe87d3 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -2648,6 +2648,19 @@ static int validate_super(struct btrfs_fs_info *fs_info, > ret = -EINVAL; > } > > + if (memcmp(fs_info->fs_devices->fsid, fs_info->super_copy->fsid, > + BTRFS_FSID_SIZE)) { > + btrfs_err(fs_info, "superblock fsid doesn't match fsid of fs_devices"); > + ret = -EINVAL; > + } > + > + if (btrfs_fs_incompat(fs_info, METADATA_UUID) && > + memcmp(fs_info->fs_devices->metadata_uuid, > + fs_info->super_copy->metadata_uuid, BTRFS_FSID_SIZE)) { > + btrfs_err(fs_info, "superblock's metadata uuid doesn't match metadata uuid of fsdevices"); > + ret = -EINVAL; > + } > + > if (memcmp(fs_info->fs_devices->metadata_uuid, sb->dev_item.fsid, > BTRFS_FSID_SIZE) != 0) { > btrfs_err(fs_info, > @@ -3279,14 +3292,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device > > disk_super = fs_info->super_copy; > > - ASSERT(!memcmp(fs_info->fs_devices->fsid, fs_info->super_copy->fsid, > - BTRFS_FSID_SIZE)); > - > - if (btrfs_fs_incompat(fs_info, METADATA_UUID)) { > - ASSERT(!memcmp(fs_info->fs_devices->metadata_uuid, > - fs_info->super_copy->metadata_uuid, > - BTRFS_FSID_SIZE)); > - } > > features = btrfs_super_flags(disk_super); > if (features & BTRFS_SUPER_FLAG_CHANGING_FSID_V2) { >
On Mon, May 31, 2021 at 12:26:01PM +0300, Nikolay Borisov wrote: > Syzbot managed to trigger this assert while performing its fuzzing. > Turns out it's better to have those asserts turned into full-fledged > checks so that in case buggy btrfs images are mounted the users gets > an error and mounting is stopped. Alternatively with CONFIG_BTRFS_ASSERT > disabled such image would have been erroneously allowed to be mounted. > > Reported-by: syzbot+a6bf271c02e4fe66b4e4@syzkaller.appspotmail.com > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Added to misc-next, thanks.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 99757939f8b0..cff694fe87d3 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -2648,6 +2648,19 @@ static int validate_super(struct btrfs_fs_info *fs_info, ret = -EINVAL; } + if (memcmp(fs_info->fs_devices->fsid, fs_info->super_copy->fsid, + BTRFS_FSID_SIZE)) { + btrfs_err(fs_info, "superblock fsid doesn't match fsid of fs_devices"); + ret = -EINVAL; + } + + if (btrfs_fs_incompat(fs_info, METADATA_UUID) && + memcmp(fs_info->fs_devices->metadata_uuid, + fs_info->super_copy->metadata_uuid, BTRFS_FSID_SIZE)) { + btrfs_err(fs_info, "superblock's metadata uuid doesn't match metadata uuid of fsdevices"); + ret = -EINVAL; + } + if (memcmp(fs_info->fs_devices->metadata_uuid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) { btrfs_err(fs_info, @@ -3279,14 +3292,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device disk_super = fs_info->super_copy; - ASSERT(!memcmp(fs_info->fs_devices->fsid, fs_info->super_copy->fsid, - BTRFS_FSID_SIZE)); - - if (btrfs_fs_incompat(fs_info, METADATA_UUID)) { - ASSERT(!memcmp(fs_info->fs_devices->metadata_uuid, - fs_info->super_copy->metadata_uuid, - BTRFS_FSID_SIZE)); - } features = btrfs_super_flags(disk_super); if (features & BTRFS_SUPER_FLAG_CHANGING_FSID_V2) {
Syzbot managed to trigger this assert while performing its fuzzing. Turns out it's better to have those asserts turned into full-fledged checks so that in case buggy btrfs images are mounted the users gets an error and mounting is stopped. Alternatively with CONFIG_BTRFS_ASSERT disabled such image would have been erroneously allowed to be mounted. Reported-by: syzbot+a6bf271c02e4fe66b4e4@syzkaller.appspotmail.com Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/disk-io.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)