2022-02-02 09:49:09

by Theodore Ts'o

[permalink] [raw]
Subject: Re: ext4's dependency on crc32c

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


2022-02-02 15:14:05

by Jan Beulich

[permalink] [raw]
Subject: Re: ext4's dependency on crc32c

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

2022-02-03 00:08:10

by Theodore Ts'o

[permalink] [raw]
Subject: Re: ext4's dependency on crc32c

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