Fix two regressions from start_creating()/start_removing() conversion

The ecryptfs_start_creating_dentry() function must be paired with the end_creating() function. Fix ecryptfs_mknod() so that end_creating() is properly called in the return path, instead of end_removing().

Fixes f046fbb4d81d ("ecryptfs: use new start_creating/start_removing APIs")

Signed-off-by Tyler Hicks <[email protected]>

---
 fs/ecryptfs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/ecryptfs/inode.c +0 -0

index 3978248247dc..e73d9de676a6 100644

2.43.0

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

Fix a mkdir-induced usage count imbalance that tripped a umount_check() BUG while unmounting the lower filesystem. Commit f046fbb4d81d ("ecryptfs: use new start_creating/start_removing APIs") added a new dget() of the lower parent dir, in ecryptfs_mkdir(), but did not dput() the dentry before returning from that function.

The BUG output as seen while running the eCryptfs test suite:

$ ./run_tests.sh -b 131072 -c safe,destructive -f ext4 -K -t lp-926292.sh
...
Running eCryptfs filesystem tests on ext4
lp-926292
------------[ cut here ]------------
BUG: Dentry ffff8e6692d11988{i=c,n=ECRYPTFS_FNEK_ENCRYPTED.FXZuRGZL7QAFtER.JeA46DtdKqkkQx9H2Vpmv234J5CU8YSsrUwZJK4AbXbrN5WkZ348wnqstovKKxA-}  still in use (1) [unmount of ext4 loop0]
WARNING: CPU: 7 PID: 950 at fs/dcache.c:1590 umount_check+0x5e/0x80
Modules linked in: md5 libmd5 ecryptfs encrypted_keys ext4 crc16 mbcache jbd2
CPU: 7 UID: 0 PID: 950 Comm: umount Not tainted 6.18.0-rc1-00013-gf046fbb4d81d #17 PREEMPT(full)
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
RIP: 0010:umount_check+0x5e/0x80
Code: 88 38 06 00 00 48 8b 40 28 4c 8b 08 48 8b 46 68 48 85 c0 74 04 48 8b 50 38 51 48 c7 c7 60 32 9c b5 48 89 f1 e8 43 5e ca ff 90 <0f> 0b 90 90 58 31 c0 e9 46 9d 6c 00 41 83 f8 01 75 b8 eb a3 66 66
RSP: 0018:ffffa19940c4bdd0 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff8e6692fad4c0 RCX: 0000000000000000
RDX: 0000000000000004 RSI: ffffa19940c4bc70 RDI: 00000000ffffffff
RBP: ffffffffb4eb5930 R08: 00000000ffffdfff R09: 0000000000000001
R10: 00000000ffffdfff R11: ffffffffb5c8a9e0 R12: ffff8e6692fad4c0
R13: ffff8e6692fad4c0 R14: ffff8e6692d11a40 R15: ffff8e6692d11988
FS:  00007f6b4b491800(0000) GS:ffff8e670506e000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f6b4b5f8d40 CR3: 0000000114eb7001 CR4: 0000000000772ef0
PKRU: 55555554
Call Trace:
 <TASK>
 d_walk+0xfd/0x370
 shrink_dcache_for_umount+0x4d/0x140
 generic_shutdown_super+0x20/0x160
 kill_block_super+0x1a/0x40
 ext4_kill_sb+0x22/0x40 [ext4]
 deactivate_locked_super+0x33/0xa0
 cleanup_mnt+0xba/0x150
 task_work_run+0x5c/0xa0
 exit_to_user_mode_loop+0xac/0xb0
 do_syscall_64+0x2ab/0xfa0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f6b4b6c2a2b
Code: c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 f3 0f 1e fa 31 f6 e9 05 00 00 00 0f 1f 44 00 00 f3 0f 1e fa b8 a6 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 b9 83 0d 00 f7 d8
RSP: 002b:00007ffcd5b8b498 EFLAGS: 00000246 ORIG_RAX: 00000000000000a6
RAX: 0000000000000000 RBX: 000055b84af0b9e0 RCX: 00007f6b4b6c2a2b
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000055b84af0bdf0
RBP: 00007ffcd5b8b570 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000103 R11: 0000000000000246 R12: 000055b84af0bae0
R13: 0000000000000000 R14: 000055b84af0bdf0 R15: 0000000000000000
 </TASK>
---[ end trace 0000000000000000 ]---
EXT4-fs (loop0): unmounting filesystem 00d9ea41-f61e-43d0-a449-6be03e7e8428.
EXT4-fs (loop0): sb orphan head is 12
sb_info orphan list:
  inode loop0:12 at ffff8e66950e1df0: mode 40700, nlink 0, next 0
Assertion failure in ext4_put_super() at fs/ext4/super.c:1345: 'list_empty(&sbi->s_orphan)'

Fixes f046fbb4d81d ("ecryptfs: use new start_creating/start_removing APIs")

Signed-off-by Tyler Hicks <[email protected]>

---
 fs/ecryptfs/inode.c | 1 +
 1 file changed, 1 insertion(+)

fs/ecryptfs/inode.c +1 -1

index e73d9de676a6..8ab014db3e03 100644
@@ -533,6 +533,7 @@ static struct dentry *ecryptfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
        fsstack_copy_inode_size(dir, lower_dir);
        set_nlink(dir, lower_dir->i_nlink);
 out:
+       dput(lower_dir_dentry);
        end_creating(lower_dentry);
        if (d_really_is_negative(dentry))
                d_drop(dentry);
--

2.43.0

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

[...]

Quoted 9 lines · Tyler Hicks

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

Ouch!

Christian,

In retrospect, it's a shame that patches get merged with zero test coverage and no ACK from the maintainer.

OTOH, relying on ACKs from all fs maintainers will seriously impair the ability to make vfs wide changes like this one.

Feels like we need to find a better balance.

At least for ecryptfs, if we know that Tyler is at least testing rc1 regularly (?) that's a comfort.

Thanks,
Amir.

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

Tyler

Tyler Hicks (2):
ecryptfs: Fix improper mknod pairing of
start_creating()/end_removing()
ecryptfs: Release lower parent dentry after creating dir

 fs/ecryptfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 10 lines · Tyler Hicks

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

Tyler

Quoted 7 lines

Tyler Hicks (2):
ecryptfs: Fix improper mknod pairing of
start_creating()/end_removing()
ecryptfs: Release lower parent dentry after creating dir

 fs/ecryptfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Thanks for finding and fixing these.
both

Reviewed-by NeilBrown <[email protected]>

I note that in https://lore.kernel.org/all/ZCuSLNnFQEdOHW0c@sequoia/ you said of ecryptfs:

I'll send a patch to deprecate and mark for removal in 2025.

Did it ever get marked for removal? Is there a chance that it might be removed?

Quoted 24 lines · NeilBrown

Quoted 10 lines · Tyler Hicks

When running the eCryptfs test suite on v6.19-rc2, I noticed BUG splats from every test and that the umount utility was segfaulting when tearing down after a test. Bisection led me to commit f046fbb4d81d ("ecryptfs:
use new start_creating/start_removing APIs").

This patch series addresses that regression and also a mknod problem spotted during code review.

Tyler

Quoted 7 lines

Tyler Hicks (2):
ecryptfs: Fix improper mknod pairing of
start_creating()/end_removing()
ecryptfs: Release lower parent dentry after creating dir

 fs/ecryptfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Thanks for finding and fixing these.
both

Reviewed-by NeilBrown <[email protected]>

Quoted 7 lines

I note that in https://lore.kernel.org/all/ZCuSLNnFQEdOHW0c@sequoia/ you said of ecryptfs:

I'll send a patch to deprecate and mark for removal in 2025.

Did it ever get marked for removal? Is there a chance that it might be removed?

If it does get removed I wonder how I and other users would access my ecryptfs folders?

It sounds to me like the road to deprecation should go through creating a FUSE alternative in ecryptfs-utils, before the kernel driver is deprecated.

Tyler, are there any problems with doing that?
I could give it a shot if I have your blessing.

Thanks,
Amir.