| Message ID | 20210621164901.1809010-1-tytso@mit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | generic/556: add a check to make sure ext4 supports encrypted casefolding | expand |
On Mon, Jun 21, 2021 at 12:49:01PM -0400, Theodore Ts'o wrote: > Some older kernels support ext4 file systems with encryption enabled, > and with casefold enabled, but not file systems have both encryption > *and* casefolding enabled. On those kernels, generic/556 will fail. > Fortunately, we can test if ext4 supports encrypted casefold via the > presence of /sys/fs/ext4/features/encrypted_casefold. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > --- > common/casefold | 17 +++++++++++++++++ > tests/generic/556 | 1 + > 2 files changed, 18 insertions(+) > > diff --git a/common/casefold b/common/casefold > index 9172d818..7b76b986 100644 > --- a/common/casefold > +++ b/common/casefold > @@ -43,6 +43,23 @@ _require_scratch_casefold() > _require_command "$LSATTR_PROG" lsattr > } > > +_require_encrypted_casefold () > +{ > + case $FSTYP in > + ext4) > + if test ! -f /sys/fs/ext4/features/casefold ; then > + _notrun "casefolding not supported" > + fi > + if test ! -f /sys/fs/ext4/features/encryption ; then > + _notrun "file system encryption not supported" > + fi > + if test ! -f /sys/fs/ext4/features/encrypted_casefold ; then > + _notrun "encrypted casefolding not supported" Dumb question: Are there kernels that support encrypted casefold but not those two components separately? --D > + fi > + ;; > + esac > +} > + > _scratch_mkfs_casefold() > { > case $FSTYP in > diff --git a/tests/generic/556 b/tests/generic/556 > index 3145188c..7916a08e 100755 > --- a/tests/generic/556 > +++ b/tests/generic/556 > @@ -16,6 +16,7 @@ status=1 # failure is thea default > . ./common/attr > > _supported_fs generic > +_require_encrypted_casefold > _require_scratch_nocheck > _require_scratch_casefold > _require_symlinks > -- > 2.31.0 >
On Mon, Jun 21, 2021 at 12:49:01PM -0400, Theodore Ts'o wrote: > diff --git a/tests/generic/556 b/tests/generic/556 > index 3145188c..7916a08e 100755 > --- a/tests/generic/556 > +++ b/tests/generic/556 > @@ -16,6 +16,7 @@ status=1 # failure is thea default > . ./common/attr > > _supported_fs generic > +_require_encrypted_casefold This isn't an encrypt+casefold test though, but rather just a casefold test. It only becomes an encrypt+casefold test when $MOUNT_OPTIONS includes test_dummy_encryption. I think we shouldn't update the test itself, but rather make _require_scratch_casefold() call _require_encrypted_casefold() if test_dummy_encryption is in $MOUNT_OPTIONS. - Eric
On Mon, Jun 21, 2021 at 10:37:58AM -0700, Eric Biggers wrote: > On Mon, Jun 21, 2021 at 12:49:01PM -0400, Theodore Ts'o wrote: > > diff --git a/tests/generic/556 b/tests/generic/556 > > index 3145188c..7916a08e 100755 > > --- a/tests/generic/556 > > +++ b/tests/generic/556 > > @@ -16,6 +16,7 @@ status=1 # failure is thea default > > . ./common/attr > > > > _supported_fs generic > > +_require_encrypted_casefold > > This isn't an encrypt+casefold test though, but rather just a casefold test. It > only becomes an encrypt+casefold test when $MOUNT_OPTIONS includes > test_dummy_encryption. > > I think we shouldn't update the test itself, but rather make > _require_scratch_casefold() call _require_encrypted_casefold() if > test_dummy_encryption is in $MOUNT_OPTIONS. > Hi Ted, just to follow up on this patch which never got merged: I tried to reproduce the test failure with the 5.4 and 5.10 kernels, whose ext4 supported casefold but not encrypt+casefold. However, it does *not* reproduce with the "ext4/encrypt" configuration of kvm-xfstests, since _require_scratch_casefold() already detects that the filesystem cannot mounted with both the encrypt and casefold features: if ! _try_scratch_mount &>>seqres.full; then _notrun "kernel can't mount filesystem with the encoding set by userspace" fi I think the problem is that you were running xfstests with test_dummy_encryption but without adding "-O encrypt" to EXT_MKFS_OPTIONS. That sort of works because of the following code in __ext4_fill_super() in the kernel which force-enables the encrypt feature if the test_dummy_encryption mount option was given: if (DUMMY_ENCRYPTION_ENABLED(sbi) && !sb_rdonly(sb) && !ext4_has_feature_encrypt(sb)) { ext4_set_feature_encrypt(sb); ext4_commit_super(sb); } However, I don't think ext4 should be doing this. The kernel should generally not be messing with the feature flags, and this code can force-enable 'encrypt' in cases where e2fsprogs would *not* allow it (e.g. the encrypt+casefold situation being encountered here). Also, f2fs doesn't allow this. So I am planning to send a kernel patch to remove the above code from ext4. - Eric
diff --git a/common/casefold b/common/casefold index 9172d818..7b76b986 100644 --- a/common/casefold +++ b/common/casefold @@ -43,6 +43,23 @@ _require_scratch_casefold() _require_command "$LSATTR_PROG" lsattr } +_require_encrypted_casefold () +{ + case $FSTYP in + ext4) + if test ! -f /sys/fs/ext4/features/casefold ; then + _notrun "casefolding not supported" + fi + if test ! -f /sys/fs/ext4/features/encryption ; then + _notrun "file system encryption not supported" + fi + if test ! -f /sys/fs/ext4/features/encrypted_casefold ; then + _notrun "encrypted casefolding not supported" + fi + ;; + esac +} + _scratch_mkfs_casefold() { case $FSTYP in diff --git a/tests/generic/556 b/tests/generic/556 index 3145188c..7916a08e 100755 --- a/tests/generic/556 +++ b/tests/generic/556 @@ -16,6 +16,7 @@ status=1 # failure is thea default . ./common/attr _supported_fs generic +_require_encrypted_casefold _require_scratch_nocheck _require_scratch_casefold _require_symlinks
Some older kernels support ext4 file systems with encryption enabled, and with casefold enabled, but not file systems have both encryption *and* casefolding enabled. On those kernels, generic/556 will fail. Fortunately, we can test if ext4 supports encrypted casefold via the presence of /sys/fs/ext4/features/encrypted_casefold. Signed-off-by: Theodore Ts'o <tytso@mit.edu> --- common/casefold | 17 +++++++++++++++++ tests/generic/556 | 1 + 2 files changed, 18 insertions(+)