On 17/11/2022 16:03, Xiubo Li wrote:
On 16/11/2022 23:37, Luís Henriques wrote:
When setting a directory's crypt context, __ceph_dir_clear_complete()
needs
to be used otherwise, if it was complete before, any old dentry
that's still
around will be valid.
Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
---
Hi!
Here's a simple way to trigger the bug this patch is fixing:
# cd /cephfs
# ls mydir
nKRhofOAVNsAwVLvDw7a0c9ypsjbZfK3n0Npnmni6j0
# ls mydir/nKRhofOAVNsAwVLvDw7a0c9ypsjbZfK3n0Npnmni6j0/
Cyuer5xT+kBlEPgtwAqSj0WK2taEljP5vHZ,D8VXCJ8
u+46b2XVCt7Obpz0gznZyNLRj79Q2l4KmkwbKOzdQKw
# fscrypt unlock mydir
# touch /mnt/test/mydir/mysubdir/file
touch: cannot touch '/mnt/test/mydir/mysubdir/file': No such file or
directory
fs/ceph/crypto.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 35a2ccfe6899..dc1557967032 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -87,6 +87,10 @@ static int ceph_crypt_get_context(struct inode
*inode, void *ctx, size_t len)
return -ERANGE;
memcpy(ctx, cfa->cfa_blob, ctxlen);
+
+ /* Directory isn't complete anymore */
+ if (S_ISDIR(inode->i_mode) && __ceph_dir_is_complete(ci))
+ __ceph_dir_clear_complete(ci);
Hi Luis,
Good catch!
BTW, why do this in the ceph_crypt_get_context() ? As my understanding
is that we should mark 'mydir' as incomplete when unlocking it. While
as I remembered the unlock operation will do:
Step1: get_encpolicy via 'mydir' as ctx
Step2: rm_enckey of ctx from the superblock
Sorry, it should be add_enckey.
Since I am still running the test cases for the file lock patches, so
I didn't catch logs to confirm the above steps yet.
If I am right IMO then we should mark the dir as incomplete in the
Step2 instead, because for non-unlock operations they may also do the
Step1.
Your patch will work. But probably we should do this just around
__fscrypt_prepare_readdir() or fscrypt_prepare_readdir() instead ? We
need to detect that once the 'inode->i_crypt_info' changed then mark the
dir as incomplete.
For now for the lock operation it will evict the inode, which will help
do this for us already. But for unlock case, we need to handle it by
ourself.
Thanks!
- Xiubo
Thanks!
- Xiubo
return ctxlen;
}
