Re: [PATCH] btrfs: qgroup: verify btrfs_qgroup_inherit parameter

[David Sterba <dsterba@suse.cz>]

On Sat, Mar 02, 2024 at 06:51:58AM +1030, Qu Wenruo wrote:
> 在 2024/3/1 22:31, David Sterba 写道:
> > On Tue, Feb 27, 2024 at 01:45:35PM +1030, Qu Wenruo wrote:
> >> [BUG]
> >> Currently btrfs can create subvolume with an invalid qgroup inherit
> >> without triggering any error:
> >>
> >>   # mkfs.btrfs -O quota -f $dev
> >>   # mount $dev $mnt
> >>   # btrfs subvolume create -i 2/0 $mnt/subv1
> >>   # btrfs qgroup show -prce --sync $mnt
> >>   Qgroupid    Referenced    Exclusive   Path
> >>   --------    ----------    ---------   ----
> >>   0/5           16.00KiB     16.00KiB   <toplevel>
> >>   0/256         16.00KiB     16.00KiB   subv1
> >>
> >> [CAUSE]
> >> We only do a very basic size check for btrfs_qgroup_inherit structure,
> >> but never really verify if the values are correct.
> >>
> >> Thus in btrfs_qgroup_inherit() function, we have to skip non-existing
> >> qgroups, and never return any error.
> >>
> >> [FIX]
> >> Fix the behavior and introduce extra checks:
> >>
> >> - Introduce early check for btrfs_qgroup_inherit structure
> >>    Not only the size, but also all the qgroup ids would be verifyed.
> >>
> >>    And the timing is very early, so we can return error early.
> >>    This early check is very important for snapshot creation, as snapshot
> >>    is delayed to transaction commit.
> >>
> >> - Drop support for btrfs_qgroup_inherit::num_ref_copies and
> >>    num_excl_copies
> >>    Those two members are used to specify to copy refr/excl numbers from
> >>    other qgroups.
> >>    This would definitely mark qgroup inconsistent, and btrfs-progs has
> >>    dropped the support for them for a long time.
> >>    It's time to drop the support for kernel.
> >>
> >> - Verify the supported btrfs_qgroup_inherit::flags
> >>    Just in case we want to add extra flags for btrfs_qgroup_inherit.
> >>
> >> Now above subvolume creation would fail with -ENOENT other than silently
> >> ignore the non-existing qgroup.
> >>
> >> CC: stable@vger.kernel.org
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >
> > Reviewed-by: David Sterba <dsterba@suse.com>
> >
> Just one thing to notice, this would cause certain test cases to fail,
> as previously any incorrect qgroup inherit would just be ignored, but
> now it would error out explicitly.

Ok, this is expected if we do fixes like that.