2022-05-01 16:50:27

by Eric Biggers

[permalink] [raw]
Subject: [xfstests PATCH 0/2] update test_dummy_encryption testing in ext4/053

This series updates the testing of the test_dummy_encryption mount
option in ext4/053.

The first patch will be needed for the test to pass if the kernel patch
"ext4: only allow test_dummy_encryption when supported"
(https://lore.kernel.org/r/[email protected])
is applied.

The second patch starts testing a case that previously wasn't tested.
It reproduces a bug that was introduced in the v5.17 kernel and will
be fixed by the kernel patch
"ext4: fix up test_dummy_encryption handling for new mount API"
(https://lore.kernel.org/r/[email protected]).

This applies on top of my recent patch
"ext4/053: fix the rejected mount option testing"
(https://lore.kernel.org/r/[email protected]).

Eric Biggers (2):
ext4/053: update the test_dummy_encryption tests
ext4/053: test changing test_dummy_encryption on remount

tests/ext4/053 | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)

--
2.36.0


2022-05-01 20:25:40

by Eric Biggers

[permalink] [raw]
Subject: [xfstests PATCH 2/2] ext4/053: test changing test_dummy_encryption on remount

From: Eric Biggers <[email protected]>

The test_dummy_encryption mount option isn't supposed to be settable or
changeable via a remount, so add test cases for this.

Signed-off-by: Eric Biggers <[email protected]>
---
tests/ext4/053 | 3 +++
1 file changed, 3 insertions(+)

diff --git a/tests/ext4/053 b/tests/ext4/053
index 84f3eab9..3d530953 100755
--- a/tests/ext4/053
+++ b/tests/ext4/053
@@ -686,6 +686,9 @@ for fstype in ext2 ext3 ext4; do
mnt test_dummy_encryption=v2
not_mnt test_dummy_encryption=bad
not_mnt test_dummy_encryption=
+ # Can't be set or changed on remount.
+ mnt_then_not_remount defaults test_dummy_encryption
+ mnt_then_not_remount test_dummy_encryption=v1 test_dummy_encryption=v2
do_mkfs -O ^encrypt $SCRATCH_DEV ${SIZE}k
fi
not_mnt test_dummy_encryption
--
2.36.0

2022-05-02 07:28:57

by Eric Biggers

[permalink] [raw]
Subject: [xfstests PATCH 1/2] ext4/053: update the test_dummy_encryption tests

From: Eric Biggers <[email protected]>

The kernel patch "ext4: only allow test_dummy_encryption when supported"
will tighten the requirements on when the test_dummy_encryption mount
option will be accepted. Update ext4/053 accordingly.

Move the test cases to later in the file to group them with the other
test cases that use do_mkfs to add custom mkfs options instead of using
the "default" filesystem that the test creates at the beginning.

Signed-off-by: Eric Biggers <[email protected]>
---
tests/ext4/053 | 35 +++++++++++++++++++++--------------
1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tests/ext4/053 b/tests/ext4/053
index bf6e3f6b..84f3eab9 100755
--- a/tests/ext4/053
+++ b/tests/ext4/053
@@ -512,20 +512,6 @@ for fstype in ext2 ext3 ext4; do
mnt noinit_itable
mnt max_dir_size_kb=4096

- if _has_kernel_config CONFIG_FS_ENCRYPTION; then
- mnt test_dummy_encryption
- mnt test_dummy_encryption=v1
- mnt test_dummy_encryption=v2
- not_mnt test_dummy_encryption=v3
- not_mnt test_dummy_encryption=
- else
- mnt test_dummy_encryption ^test_dummy_encryption
- mnt test_dummy_encryption=v1 ^test_dummy_encryption=v1
- mnt test_dummy_encryption=v2 ^test_dummy_encryption=v2
- mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3
- not_mnt test_dummy_encryption=
- fi
-
if _has_kernel_config CONFIG_FS_ENCRYPTION_INLINE_CRYPT; then
mnt inlinecrypt
else
@@ -687,6 +673,27 @@ for fstype in ext2 ext3 ext4; do
mnt_then_not_remount defaults jqfmt=vfsv1
remount defaults grpjquota=,usrjquota= ignored

+ echo "== Testing the test_dummy_encryption option" >> $seqres.full
+ # Since kernel commit "ext4: only allow test_dummy_encryption when
+ # supported", the test_dummy_encryption mount option is only allowed
+ # when the filesystem has the encrypt feature and the kernel has
+ # CONFIG_FS_ENCRYPTION. Note, the encrypt feature requirement implies
+ # that this option is never allowed on ext2 or ext3 mounts.
+ if [[ $fstype == ext4 ]] && _has_kernel_config CONFIG_FS_ENCRYPTION; then
+ do_mkfs -O encrypt $SCRATCH_DEV ${SIZE}k
+ mnt test_dummy_encryption
+ mnt test_dummy_encryption=v1
+ mnt test_dummy_encryption=v2
+ not_mnt test_dummy_encryption=bad
+ not_mnt test_dummy_encryption=
+ do_mkfs -O ^encrypt $SCRATCH_DEV ${SIZE}k
+ fi
+ not_mnt test_dummy_encryption
+ not_mnt test_dummy_encryption=v1
+ not_mnt test_dummy_encryption=v2
+ not_mnt test_dummy_encryption=bad
+ not_mnt test_dummy_encryption=
+
done #for fstype in ext2 ext3 ext4; do

$UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
--
2.36.0

2022-05-02 17:02:11

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [xfstests PATCH 1/2] ext4/053: update the test_dummy_encryption tests

On Sat, Apr 30, 2022 at 10:19:27PM -0700, Eric Biggers wrote:
>
> The kernel patch "ext4: only allow test_dummy_encryption when supported"
> will tighten the requirements on when the test_dummy_encryption mount
> option will be accepted. Update ext4/053 accordingly.

One of the problems with ext4/053 is that it is very implementation
dependent. It was useful when we were making the change to the new
mount API, but the problem is any future changes to the mount option
handling is going to break the patch.

So for example, the kernel patch which Eric has proposed, "ext4: only
allow test_dummy_encryption when supported", breaks ext4/053, which I
noted in the review the patch. But then this patch will break kernels
as they currently stand without this patch, and for kernels that
haven't moved to the new mount API, fixing it is going to be a mess.

Perhaps ext4/053 is still useful in that it will flag changes that
might unintentionally make user-visible changes mount options handling
in ext4, but for cases like this one, where we are changing a mount
option which is really intended for kernel developers, perhaps the
right approach here is to just remove the parts of ext4/053 that are
testing the behaviour of test_dummy_encryption in such a
super-nit-picky way?

What do folks think?

> Move the test cases to later in the file to group them with the other
> test cases that use do_mkfs to add custom mkfs options instead of using
> the "default" filesystem that the test creates at the beginning.

Note: this patch doesn't apply because ext4/053 currently has this
line:

not_mnt test_dummy_encryption=v3

and the patch is trying to remove this line in the first patch chunk:

mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3

I checked the upstream version of ext4/053 just in case I had some
local modification of ext4/053 in my tree via "git diff -r
origin/master tests/ext4/053" but that returned no deltas.

Eric, do you have a local modification to this test in your tree
already, perhaps?

- Ted

2022-05-02 23:32:19

by Eric Biggers

[permalink] [raw]
Subject: Re: [xfstests PATCH 1/2] ext4/053: update the test_dummy_encryption tests

On Mon, May 02, 2022 at 05:46:11AM -0700, tytso wrote:
> On Sat, Apr 30, 2022 at 10:19:27PM -0700, Eric Biggers wrote:
> >
> > The kernel patch "ext4: only allow test_dummy_encryption when supported"
> > will tighten the requirements on when the test_dummy_encryption mount
> > option will be accepted. Update ext4/053 accordingly.
>
> One of the problems with ext4/053 is that it is very implementation
> dependent. It was useful when we were making the change to the new
> mount API, but the problem is any future changes to the mount option
> handling is going to break the patch.
>
> So for example, the kernel patch which Eric has proposed, "ext4: only
> allow test_dummy_encryption when supported", breaks ext4/053, which I
> noted in the review the patch. But then this patch will break kernels
> as they currently stand without this patch, and for kernels that
> haven't moved to the new mount API, fixing it is going to be a mess.
>
> Perhaps ext4/053 is still useful in that it will flag changes that
> might unintentionally make user-visible changes mount options handling
> in ext4, but for cases like this one, where we are changing a mount
> option which is really intended for kernel developers, perhaps the
> right approach here is to just remove the parts of ext4/053 that are
> testing the behaviour of test_dummy_encryption in such a
> super-nit-picky way?
>
> What do folks think?

I'd like to keep the test_dummy_encryption test cases. Trying to add a couple
new test cases (patch 2) actually found a regression.

We could gate them on the kernel version, similar to the whole ext4/053 which
already only runs on kernel version 5.12. (Kernel versions checks suck, but
maybe it's the right choice for this very-nit-picky test.) Alternatively, I
could just backport "ext4: only allow test_dummy_encryption when supported" to
5.15, which would be the only relevant LTS kernel version.

>
> > Move the test cases to later in the file to group them with the other
> > test cases that use do_mkfs to add custom mkfs options instead of using
> > the "default" filesystem that the test creates at the beginning.
>
> Note: this patch doesn't apply because ext4/053 currently has this
> line:
>
> not_mnt test_dummy_encryption=v3
>
> and the patch is trying to remove this line in the first patch chunk:
>
> mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3
>
> I checked the upstream version of ext4/053 just in case I had some
> local modification of ext4/053 in my tree via "git diff -r
> origin/master tests/ext4/053" but that returned no deltas.
>
> Eric, do you have a local modification to this test in your tree
> already, perhaps?

Sorry about that; as I mentioned in the cover letter, this is based on my other
patch "ext4/053: fix the rejected mount option testing". As-is, 'not_mnt'
doesn't really work at all, so I wanted to fix that first.

- Eric