linux-ext4 - Re: [PATCH v3 1/6] common/rc: fix btrfs mixed mode usage in _scratch_mkfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 Feb 2022 07:02:30 +0000
From:   Naohiro Aota <Naohiro.Aota@....com>
To:     Eryu Guan <guan@...u.me>
CC:     Shinichiro Kawasaki <shinichiro.kawasaki@....com>,
        "fstests@...r.kernel.org" <fstests@...r.kernel.org>,
        "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        Johannes Thumshirn <Johannes.Thumshirn@....com>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>
Subject: Re: [PATCH v3 1/6] common/rc: fix btrfs mixed mode usage in
 _scratch_mkfs_sized

On Mon, Feb 21, 2022 at 01:00:23AM +0800, Eryu Guan wrote:
> On Fri, Feb 18, 2022 at 04:31:51PM +0900, Shin'ichiro Kawasaki wrote:
> > The helper function _scratch_mkfs_sized needs a couple of improvements
> > for btrfs. At first, the function adds --mixed option to mkfs.btrfs when
> > the filesystem size is smaller then 256MiB, but this threshold is no
> > longer correct and it should be 109MiB. Secondly, the --mixed option
> 
> I'm wondering if this 256M -> 109M change was made just recently or was
> made on old kernel.

The check is imposed from the userland tool btrfs-progs. The value is
calculated from a code in 31d228a2eb98 ("btrfs-progs: mkfs: Enhance
minimal device size calculation to fix mkfs failure on small file"),
which is released around v4.14.

But, after rechecking the code, the size part of the patch looks
invalid to me. My bad.

https://github.com/kdave/btrfs-progs/blob/master/mkfs/common.c#L651

As said in 50c1905c2795 ("btrfs: _scratch_mkfs_sized fix min size
without mixed option"), we need to consider every possible profile to
decide the minimal value.

That gives me:

- reserved += BTRFS_BLOCK_RESERVED_1M_FOR_SUPER +
	    BTRFS_MKFS_SYSTEM_GROUP_SIZE + SZ_8M * 2;
  --> reserved = 1M + 4M + 8M * 2 = 21M

- meta_size = SZ_8M + SZ_32M;
- meta_size *= 2;
- reserved += meta_size;
  --> reserved = 21M + (8M + 32M) * 2 = 101M

- data_size = 64M;
- data_size *= 2;
- reserved += data_size;
  --> reserved = 101M + 64M * 2 = 229M

We can also confirm the calculation with a zero size file:

   $ mkfs.btrfs -f -d DUP -m DUP btrfs.img
   btrfs-progs v5.16 
   See http://btrfs.wiki.kernel.org for more information.
   
   ERROR: 'btrfs.img' is too small to make a usable filesystem
   ERROR: minimum size for each btrfs device is 240123904

So, the original 256MB is roughly correct.

> If it was changed just recently, say 5.14 kernel, I suspect that tests
> will fail on kernels prior to that change.
> 
> But if this change was made on some acient kernels, say 4.10, then I
> think we're fine with this patch.
> 
> Thanks,
> Eryu
> 
> > shall not be specified to mkfs.btrfs for zoned devices, since zoned
> > devices does not allow mixing metadata blocks and data blocks.
> > 
> > Suggested-by: Naohiro Aota <naohiro.aota@....com>
> > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@....com>
> > Reviewed-by: Johannes Thumshirn <johannes.thumshirn@....com>
> > ---
> >  common/rc | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/common/rc b/common/rc
> > index de60fb7b..74d2d8bd 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1075,10 +1075,10 @@ _scratch_mkfs_sized()
> >  		;;
> >  	btrfs)
> >  		local mixed_opt=
> > -		# minimum size that's needed without the mixed option.
> > -		# Ref: btrfs-prog: btrfs_min_dev_size()
> > -		# Non mixed mode is also the default option.
> > -		(( fssize < $((256 * 1024 *1024)) )) && mixed_opt='--mixed'
> > +		# Mixed option is required when the filesystem size is small and
> > +		# the device is not zoned. Ref: btrfs-progs: btrfs_min_dev_size()
> > +		(( fssize < $((109 * 1024 * 1024)) )) &&
> > +			! _scratch_btrfs_is_zoned && mixed_opt='--mixed'
> >  		$MKFS_BTRFS_PROG $MKFS_OPTIONS $mixed_opt -b $fssize $SCRATCH_DEV
> >  		;;
> >  	jfs)
> > -- 
> > 2.34.1