linux-ext4 - Re: Collapsing the number of feature flags (was Re: [PATCH v2.3 00/23] ext4: Add metadata checksumming)

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]

Message-ID: <20120209195444.GA15164@tux1.beaverton.ibm.com>
Date:	Thu, 9 Feb 2012 11:54:44 -0800
From:	"Darrick J. Wong" <djwong@...ibm.com>
To:	"Ted Ts'o" <tytso@....edu>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: Collapsing the number of feature flags (was Re: [PATCH v2.3
	00/23] ext4: Add metadata checksumming)

On Wed, Feb 08, 2012 at 01:08:47PM -0500, Ted Ts'o wrote:
> So I've started looking at this patch series, and I'm wondering if it
> might be better if we collapse these two feature flags:
> 
> 	EXT4_FEATURE_RO_COMPAT_METADATA_CSUM
> and
> 	EXT4_FEATURE_INCOMPAT_BG_USE_META_CSUM
> 
> To a single feature flag, EXT4_FEATURE_INCOMPAT_METADATA_CSUM, which
> also implies EXT4_FEATURE_RO_COMPAT_GDT_CSUM.  (So in the future, when
> we enable INCOMPAT_CSUM_METADATA, the presense or absence of
> EXT4_RO_COMPAT_GDT_CSUM won't matter, and in fact mke2fs will skip
> setting that feature flag altogether.  Tune2fs could also drop the
> GDT_CSUM flag when adding the CSUM_METADATA flag.)

I'm ok with this.

> The reasoning behind this is that it simplifies the combinatorics we
> need to test, and it also simplifies our code base.  In addition, it's
> really easy to make tune2fs recalculate the checksums when the feature
> flag is set, and reculate the block group checksums using the old
> algorithm when the metadata flag is unset.  So if someone wants to
> mount the file system on a downlevel kernel, it's really not that bad
> that this feature is an INCOMPAT feature; we can easily downgrade it
> using tune2fs.
> 
> In fact, if we wanted to take this to extremes, we could call it
> EXT4_FEATURE_INCOMPAT_NEW_METADATA, and then let it imply the
> following feature flags as well:
> 
> 	EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER
> 	EXT4_FEATURE_RO_COMPAT_LARGE_FILE
> 	EXT4_FEATURE_RO_COMPAT_HUGE_FILE
> 	EXT4_FEATURE_RO_COMPAT_DIR_NLINK
> 	EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE
> 	EXT4_FEATURE_INCOMPAT_FILETYPE
> 	EXT4_FEATURE_INCOMPAT_FLEX_BG
> 
> We can add inline functions in fs/ext4/ext4.h and in
> lib/ext2fs/ext2fs.h to make the source code look a bit simpler.
> 
> This would help to reduce our testing load, and it would also make the
> output of dumpe2fs easier to understand...

Hm... does this make it impossible to add checksums to a fs that doesn't have
flexbg set?  I'm not 100% sure what happens if you enable the flexbg bit on a
filesystem that wasn't mkfs'd with that turned on.  Most of the other flags
look like they've been mkfs default for years, but I could be wrong.

I'm concerned that implementing this second idea would make it more difficult
to add checksums to an older filesystem.

--D
> 
> 						- Ted
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html