* [PATCH] generic/395: remove workarounds for wrong error codes @ 2020-10-31 5:40 Eric Biggers 2020-10-31 17:34 ` Theodore Y. Ts'o 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2020-10-31 5:40 UTC (permalink / raw) To: fstests; +Cc: linux-fscrypt From: Eric Biggers <ebiggers@google.com> generic/395 contains workarounds to allow for some of the fscrypt ioctls to fail with different error codes. However, the error codes were all fixed up and documented years ago: - FS_IOC_GET_ENCRYPTION_POLICY on ext4 failed with ENOENT instead of ENODATA on unencrypted files. Fixed by commit db717d8e26c2 ("fscrypto: move ioctl processing more fully into common code"). - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of EEXIST on encrypted files. Fixed by commit 8488cd96ff88 ("fscrypt: use EEXIST when file already uses different policy"). - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of ENOTDIR on nondirectories. Fixed by commit dffd0cfa06d4 ("fscrypt: use ENOTDIR when setting encryption policy on nondirectory"). It's been long enough, so update the test to expect the correct behavior only, so we don't accidentally reintroduce the wrong behavior. Signed-off-by: Eric Biggers <ebiggers@google.com> --- tests/generic/395 | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/tests/generic/395 b/tests/generic/395 index 3fa2a823..34121dd9 100755 --- a/tests/generic/395 +++ b/tests/generic/395 @@ -38,31 +38,19 @@ _require_user _scratch_mkfs_encrypted &>> $seqres.full _scratch_mount -check_no_policy() -{ - # When a file is unencrypted, FS_IOC_GET_ENCRYPTION_POLICY currently - # fails with ENOENT on ext4 but with ENODATA on f2fs. TODO: it's - # planned to consistently use ENODATA. For now this test accepts both. - _get_encpolicy $1 |& - sed -e 's/No such file or directory/No data available/' -} - # Should be able to set an encryption policy on an empty directory empty_dir=$SCRATCH_MNT/empty_dir echo -e "\n*** Setting encryption policy on empty directory ***" mkdir $empty_dir -check_no_policy $empty_dir |& _filter_scratch +_get_encpolicy $empty_dir |& _filter_scratch _set_encpolicy $empty_dir 0000111122223333 _get_encpolicy $empty_dir | _filter_scratch # Should be able to set the same policy again, but not a different one. -# TODO: the error code for "already has a different policy" is planned to switch -# from EINVAL to EEXIST. For now this test accepts both. echo -e "\n*** Setting encryption policy again ***" _set_encpolicy $empty_dir 0000111122223333 _get_encpolicy $empty_dir | _filter_scratch -_set_encpolicy $empty_dir 4444555566667777 |& \ - _filter_scratch | sed -e 's/Invalid argument/File exists/' +_set_encpolicy $empty_dir 4444555566667777 |& _filter_scratch _get_encpolicy $empty_dir | _filter_scratch # Should *not* be able to set an encryption policy on a nonempty directory @@ -71,19 +59,16 @@ echo -e "\n*** Setting encryption policy on nonempty directory ***" mkdir $nonempty_dir touch $nonempty_dir/file _set_encpolicy $nonempty_dir |& _filter_scratch -check_no_policy $nonempty_dir |& _filter_scratch +_get_encpolicy $nonempty_dir |& _filter_scratch # Should *not* be able to set an encryption policy on a nondirectory file, even # an empty one. Regression test for 002ced4be642: "fscrypto: only allow setting # encryption policy on directories". -# TODO: the error code for "not a directory" is planned to switch from EINVAL to -# ENOTDIR. For now this test accepts both. nondirectory=$SCRATCH_MNT/nondirectory echo -e "\n*** Setting encryption policy on nondirectory ***" touch $nondirectory -_set_encpolicy $nondirectory |& \ - _filter_scratch | sed -e 's/Invalid argument/Not a directory/' -check_no_policy $nondirectory |& _filter_scratch +_set_encpolicy $nondirectory |& _filter_scratch +_get_encpolicy $nondirectory |& _filter_scratch # Should *not* be able to set an encryption policy on another user's directory. # Regression test for 163ae1c6ad62: "fscrypto: add authorization check for @@ -92,7 +77,7 @@ unauthorized_dir=$SCRATCH_MNT/unauthorized_dir echo -e "\n*** Setting encryption policy on another user's directory ***" mkdir $unauthorized_dir _user_do_set_encpolicy $unauthorized_dir |& _filter_scratch -check_no_policy $unauthorized_dir |& _filter_scratch +_get_encpolicy $unauthorized_dir |& _filter_scratch # Should *not* be able to set an encryption policy on a directory on a # filesystem mounted readonly. Regression test for ba63f23d69a3: "fscrypto: @@ -102,12 +87,12 @@ echo -e "\n*** Setting encryption policy on readonly filesystem ***" mkdir $SCRATCH_MNT/ro_dir $SCRATCH_MNT/ro_bind_mnt _scratch_remount ro _set_encpolicy $SCRATCH_MNT/ro_dir |& _filter_scratch -check_no_policy $SCRATCH_MNT/ro_dir |& _filter_scratch +_get_encpolicy $SCRATCH_MNT/ro_dir |& _filter_scratch _scratch_remount rw mount --bind $SCRATCH_MNT $SCRATCH_MNT/ro_bind_mnt mount -o remount,ro,bind $SCRATCH_MNT/ro_bind_mnt _set_encpolicy $SCRATCH_MNT/ro_bind_mnt/ro_dir |& _filter_scratch -check_no_policy $SCRATCH_MNT/ro_bind_mnt/ro_dir |& _filter_scratch +_get_encpolicy $SCRATCH_MNT/ro_bind_mnt/ro_dir |& _filter_scratch umount $SCRATCH_MNT/ro_bind_mnt # success, all done -- 2.29.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] generic/395: remove workarounds for wrong error codes 2020-10-31 5:40 [PATCH] generic/395: remove workarounds for wrong error codes Eric Biggers @ 2020-10-31 17:34 ` Theodore Y. Ts'o 2020-10-31 18:10 ` Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: Theodore Y. Ts'o @ 2020-10-31 17:34 UTC (permalink / raw) To: Eric Biggers; +Cc: fstests, linux-fscrypt On Fri, Oct 30, 2020 at 10:40:18PM -0700, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > generic/395 contains workarounds to allow for some of the fscrypt ioctls > to fail with different error codes. However, the error codes were all > fixed up and documented years ago: > > - FS_IOC_GET_ENCRYPTION_POLICY on ext4 failed with ENOENT instead of > ENODATA on unencrypted files. Fixed by commit db717d8e26c2 > ("fscrypto: move ioctl processing more fully into common code"). > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of EEXIST > on encrypted files. Fixed by commit 8488cd96ff88 ("fscrypt: use > EEXIST when file already uses different policy"). > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of ENOTDIR > on nondirectories. Fixed by commit dffd0cfa06d4 ("fscrypt: use > ENOTDIR when setting encryption policy on nondirectory"). > > It's been long enough, so update the test to expect the correct behavior > only, so we don't accidentally reintroduce the wrong behavior. > > Signed-off-by: Eric Biggers <ebiggers@google.com> LGTM Did these fixes get backported into the stable kernels (and the relevant Android trees)? - Ted ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] generic/395: remove workarounds for wrong error codes 2020-10-31 17:34 ` Theodore Y. Ts'o @ 2020-10-31 18:10 ` Eric Biggers 2020-11-09 23:40 ` Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2020-10-31 18:10 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: fstests, linux-fscrypt On Sat, Oct 31, 2020 at 01:34:39PM -0400, Theodore Y. Ts'o wrote: > On Fri, Oct 30, 2020 at 10:40:18PM -0700, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > generic/395 contains workarounds to allow for some of the fscrypt ioctls > > to fail with different error codes. However, the error codes were all > > fixed up and documented years ago: > > > > - FS_IOC_GET_ENCRYPTION_POLICY on ext4 failed with ENOENT instead of > > ENODATA on unencrypted files. Fixed by commit db717d8e26c2 > > ("fscrypto: move ioctl processing more fully into common code"). > > > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of EEXIST > > on encrypted files. Fixed by commit 8488cd96ff88 ("fscrypt: use > > EEXIST when file already uses different policy"). > > > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of ENOTDIR > > on nondirectories. Fixed by commit dffd0cfa06d4 ("fscrypt: use > > ENOTDIR when setting encryption policy on nondirectory"). > > > > It's been long enough, so update the test to expect the correct behavior > > only, so we don't accidentally reintroduce the wrong behavior. > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > LGTM > > Did these fixes get backported into the stable kernels (and the > relevant Android trees)? > Some of them. Regarding stable kernels, currently if these 3 xfstests patches are applied, generic/395 will fail on 4.9 and earlier, generic/397 will fail on ubifs on 4.19 and earlier, and generic/398 will fail on 4.19 and earlier. In Android kernels, the fscrypt support tends to be somewhat more up-to-date than in the corresponding LTS kernels, as the latest fscrypt-related patches were backported to them while they were open for development. E.g., the latest 3.18, 4.4, and 4.9 Android common kernels have fs/crypto/ at the equivalent of upstream 4.17 or 4.18. Those branches are closed for development though, so they won't be getting anything newer than that except through LTS. (And devices using those kernel versions don't necessarily get kernel updates anymore.) Backporting these patches can be tricky since the fscrypt code has changed a lot, so in most cases they would require writing custom backports. So there's only so much I can do about older kernels. But probably the most important patch I should backport to LTS is f5e55e777cc9 ("fscrypt: return -EXDEV for incompatible rename or link into encrypted dir"), as that would get the tests passing on ext4 and f2fs on 4.14 and 4.19, and that patch was a fix for a bug that was causing problems for people. - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] generic/395: remove workarounds for wrong error codes 2020-10-31 18:10 ` Eric Biggers @ 2020-11-09 23:40 ` Eric Biggers 2020-11-10 4:41 ` Eric Biggers 0 siblings, 1 reply; 5+ messages in thread From: Eric Biggers @ 2020-11-09 23:40 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: fstests, linux-fscrypt On Sat, Oct 31, 2020 at 11:10:50AM -0700, Eric Biggers wrote: > On Sat, Oct 31, 2020 at 01:34:39PM -0400, Theodore Y. Ts'o wrote: > > On Fri, Oct 30, 2020 at 10:40:18PM -0700, Eric Biggers wrote: > > > From: Eric Biggers <ebiggers@google.com> > > > > > > generic/395 contains workarounds to allow for some of the fscrypt ioctls > > > to fail with different error codes. However, the error codes were all > > > fixed up and documented years ago: > > > > > > - FS_IOC_GET_ENCRYPTION_POLICY on ext4 failed with ENOENT instead of > > > ENODATA on unencrypted files. Fixed by commit db717d8e26c2 > > > ("fscrypto: move ioctl processing more fully into common code"). > > > > > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of EEXIST > > > on encrypted files. Fixed by commit 8488cd96ff88 ("fscrypt: use > > > EEXIST when file already uses different policy"). > > > > > > - FS_IOC_SET_ENCRYPTION_POLICY failed with EINVAL instead of ENOTDIR > > > on nondirectories. Fixed by commit dffd0cfa06d4 ("fscrypt: use > > > ENOTDIR when setting encryption policy on nondirectory"). > > > > > > It's been long enough, so update the test to expect the correct behavior > > > only, so we don't accidentally reintroduce the wrong behavior. > > > > > > Signed-off-by: Eric Biggers <ebiggers@google.com> > > > > LGTM > > > > Did these fixes get backported into the stable kernels (and the > > relevant Android trees)? > > > > Some of them. Regarding stable kernels, currently if these 3 xfstests patches > are applied, generic/395 will fail on 4.9 and earlier, generic/397 will fail on > ubifs on 4.19 and earlier, and generic/398 will fail on 4.19 and earlier. > > In Android kernels, the fscrypt support tends to be somewhat more up-to-date > than in the corresponding LTS kernels, as the latest fscrypt-related patches > were backported to them while they were open for development. E.g., the latest > 3.18, 4.4, and 4.9 Android common kernels have fs/crypto/ at the equivalent of > upstream 4.17 or 4.18. Those branches are closed for development though, so > they won't be getting anything newer than that except through LTS. (And devices > using those kernel versions don't necessarily get kernel updates anymore.) > > Backporting these patches can be tricky since the fscrypt code has changed a > lot, so in most cases they would require writing custom backports. > > So there's only so much I can do about older kernels. > > But probably the most important patch I should backport to LTS is f5e55e777cc9 > ("fscrypt: return -EXDEV for incompatible rename or link into encrypted dir"), > as that would get the tests passing on ext4 and f2fs on 4.14 and 4.19, and that > patch was a fix for a bug that was causing problems for people. > I ended up backporting some of the missing patches to some of the LTS kernels. Now the status of the "encrypt" group tests is: 5.10-rc3: all pass, but generic/602 is flaky on ext4, which will be fixed by https://lkml.kernel.org/linux-fscrypt/20201109231151.GB853@sol.localdomain 5.4: all pass. 4.19: all pass since v4.19.155. 4.14: all pass on ext4 and f2fs since v4.14.204. generic/{397,398,429} still fail on ubifs; it's hard to backport the needed patches to 4.14. 4.9: all pass on ext4 since v4.9.242 (not officially released yet). generic/547 still fails on f2fs due to a mysterious bug that causes dump.f2fs to not show the xattrs. ubifs encryption wasn't supported yet. 4.4: generic/{395,397} still fail on ext4, and generic/{395,397,398,419,429,440} still fail on f2fs. ubifs encryption wasn't supported yet. - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] generic/395: remove workarounds for wrong error codes 2020-11-09 23:40 ` Eric Biggers @ 2020-11-10 4:41 ` Eric Biggers 0 siblings, 0 replies; 5+ messages in thread From: Eric Biggers @ 2020-11-10 4:41 UTC (permalink / raw) To: Theodore Y. Ts'o; +Cc: fstests, linux-fscrypt On Mon, Nov 09, 2020 at 03:40:51PM -0800, Eric Biggers wrote: > > I ended up backporting some of the missing patches to some of the LTS kernels. > > Now the status of the "encrypt" group tests is: > > 5.10-rc3: all pass, but generic/602 is flaky on ext4, which will be fixed by > https://lkml.kernel.org/linux-fscrypt/20201109231151.GB853@sol.localdomain > > 5.4: all pass. > Correction: there are two more test failures on upstream and on 5.4. generic/580 fails on f2fs due to the lazytime bug (https://lkml.kernel.org/r/20200306004555.GB225345@gmail.com), and generic/595 fails on ubifs due to a longstanding race condition where a file can be created using a negative "no-key" dentry. I'm planning to fix these. - Eric ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-10 4:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-31 5:40 [PATCH] generic/395: remove workarounds for wrong error codes Eric Biggers 2020-10-31 17:34 ` Theodore Y. Ts'o 2020-10-31 18:10 ` Eric Biggers 2020-11-09 23:40 ` Eric Biggers 2020-11-10 4:41 ` Eric Biggers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).