ext4's dependency on crc32c

ext4's dependency on crc32c

[Jan Beulich]

Hello,

in 5.16, due to (afaict) adad556efcdd ("crypto: api - Fix built-in
testing dependency failures") booting a system with cryptmgr.ko not
(perhaps manually) put in the initrd doesn't work when ext4.ko is
responsible for / . I've contacted Herbert already after finding
this issue with btrfs, but in the case of ext4 another aspect plays
into it: I've observed the problem on a system where ext4.ko is used
solely to service ext3 partitions (including / ), but aiui crc32c
isn't used at all in this case. Yet it's the attempt of loading it
which actually causes the mount (and hence booting) to fail.

If my understanding is correct, wouldn't it make sense to skip the
call to crypto_alloc_shash() unless an ext4 superblock is being
processed?

Thanks, Jan

Re: ext4's dependency on crc32c

[Theodore Ts'o]

On Tue, Feb 01, 2022 at 03:19:54PM +0100, Jan Beulich wrote:
> Hello,
> 
> in 5.16, due to (afaict) adad556efcdd ("crypto: api - Fix built-in
> testing dependency failures") booting a system with cryptmgr.ko not
> (perhaps manually) put in the initrd doesn't work when ext4.ko is
> responsible for / . I've contacted Herbert already after finding
> this issue with btrfs, but in the case of ext4 another aspect plays
> into it: I've observed the problem on a system where ext4.ko is used
> solely to service ext3 partitions (including / ), but aiui crc32c
> isn't used at all in this case. Yet it's the attempt of loading it
> which actually causes the mount (and hence booting) to fail.
> 
> If my understanding is correct, wouldn't it make sense to skip the
> call to crypto_alloc_shash() unless an ext4 superblock is being
> processed?

Sure, there are some subtleties, though.  For example, we would need
to make sure that sbi->s_chksum_driver() is initialized before we
attempt to use it.  That's because an malicious attacker (or syzbot
fuzzer --- is there a difference? :-) could force the file system
feature bits to be set after we decide whether or not to allocate the
crypto handle.  This can happen by having a maliciously corrupted file
system image which sets the file system feature bits as part of the
journal replay, or simply by writing to the superblock after it is
mounted.

	       	       				- Ted

Re: ext4's dependency on crc32c

[Jan Beulich]

On 01.02.2022 16:47, Theodore Ts'o wrote:
> On Tue, Feb 01, 2022 at 03:19:54PM +0100, Jan Beulich wrote:
>> Hello,
>>
>> in 5.16, due to (afaict) adad556efcdd ("crypto: api - Fix built-in
>> testing dependency failures") booting a system with cryptmgr.ko not
>> (perhaps manually) put in the initrd doesn't work when ext4.ko is
>> responsible for / . I've contacted Herbert already after finding
>> this issue with btrfs, but in the case of ext4 another aspect plays
>> into it: I've observed the problem on a system where ext4.ko is used
>> solely to service ext3 partitions (including / ), but aiui crc32c
>> isn't used at all in this case. Yet it's the attempt of loading it
>> which actually causes the mount (and hence booting) to fail.
>>
>> If my understanding is correct, wouldn't it make sense to skip the
>> call to crypto_alloc_shash() unless an ext4 superblock is being
>> processed?
> 
> Sure, there are some subtleties, though.  For example, we would need
> to make sure that sbi->s_chksum_driver() is initialized before we
> attempt to use it.  That's because an malicious attacker (or syzbot
> fuzzer --- is there a difference? :-) could force the file system
> feature bits to be set after we decide whether or not to allocate the
> crypto handle.  This can happen by having a maliciously corrupted file
> system image which sets the file system feature bits as part of the
> journal replay, or simply by writing to the superblock after it is
> mounted.

Can any of this happen for an ext3 partition (without destroying its
ext3 nature)? IOW would it be possible to set sbi->s_chksum_driver
depending on just file system type rather than individual features?

Jan

Re: ext4's dependency on crc32c

[Theodore Ts'o]

On Wed, Feb 02, 2022 at 09:08:03AM +0100, Jan Beulich wrote:
> > Sure, there are some subtleties, though.  For example, we would need
> > to make sure that sbi->s_chksum_driver() is initialized before we
> > attempt to use it.  That's because an malicious attacker (or syzbot
> > fuzzer --- is there a difference? :-) could force the file system
> > feature bits to be set after we decide whether or not to allocate the
> > crypto handle.  This can happen by having a maliciously corrupted file
> > system image which sets the file system feature bits as part of the
> > journal replay, or simply by writing to the superblock after it is
> > mounted.
> 
> Can any of this happen for an ext3 partition (without destroying its
> ext3 nature)? IOW would it be possible to set sbi->s_chksum_driver
> depending on just file system type rather than individual features?

The idea of "an ext3 partition" is not well defined, at least in terms
of the on-disk format.  The ext2/ext3/ext4 superblock has a set of
feature flags, the compat, r/o, and incompat feature flags.  You can
take an "ext3" file system, and enable the "extents" feature, and on
modern kernels (where "mount -t ext3" is handled by the ext4 file
system), new files which are created will be extent-mapped.

You can look at what "mke2fs -t ext3" and "mke2fs -t ext4" will do ---
although that will change over time as you install new versions of
e2fsprogs, but it can also be modified by editing the /etc/mke2fs.conf
file, either becaue a distribution wants to be more aggressive about
enabling a bleeding edge feature (such as fast commits), or because a
particular system adminsitrator or company wants to explicitly enable
or disable some features for their workload:

[defaults]
	base_features = sparse_super,large_file,filetype,resize_inode,dir_index,ext_attr
	default_mntopts = acl,user_xattr
	enable_periodic_fsck = 0
	blocksize = 4096
	inode_size = 256
	...
	
[fs_types]
	ext3 = {
		features = has_journal
	}
	ext4 = {
		features = has_journal,extent,huge_file,flex_bg,metadata_csum,64bit,dir_nlink,extra_isize
	}
	....

Cheers,

					- Ted