Re: [PATCH v3] ceph: mark directory as non-complete complete after loading key

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 23/11/2022 17:59, Luís Henriques wrote:
Xiubo Li <xiubli@xxxxxxxxxx> writes:

On 22/11/2022 22:59, Luís Henriques wrote:
When setting a directory's crypt context, ceph_dir_clear_complete() needs to
be called otherwise if it was complete before, any existing (old) dentry will
still be valid.

This patch adds a wrapper around __fscrypt_prepare_readdir() which will
ensure a directory is marked as non-complete if key status changes.

Signed-off-by: Luís Henriques <lhenriques@xxxxxxx>
---
Hi!

Hopefully I'm not breaking anything else.  I've run a bunch of fstests and
didn't saw any (new) failures.

Changes since v2:
- Created helper wrapper for __fscrypt_prepare_readdir()
- Added calls to the new helper

Changes since v1:
- Moved the __ceph_dir_clear_complete() call from ceph_crypt_get_context()
    to ceph_lookup().
- Added an __fscrypt_prepare_readdir() wrapper to check key status changes

   fs/ceph/crypto.c     | 35 +++++++++++++++++++++++++++++++++--
   fs/ceph/crypto.h     |  6 ++++++
   fs/ceph/dir.c        |  8 ++++----
   fs/ceph/mds_client.c |  6 +++---
   4 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/fs/ceph/crypto.c b/fs/ceph/crypto.c
index 35a2ccfe6899..5ef65a06af98 100644
--- a/fs/ceph/crypto.c
+++ b/fs/ceph/crypto.c
@@ -397,8 +397,8 @@ int ceph_fname_to_usr(const struct ceph_fname *fname, struct fscrypt_str *tname,
   		goto out_inode;
   	}
   -	ret = __fscrypt_prepare_readdir(dir);
-	if (ret)
+	ret = ceph_fscrypt_prepare_readdir(dir);
+	if (ret < 0)
   		goto out_inode;
     	/*
@@ -636,3 +636,34 @@ int ceph_fscrypt_encrypt_pages(struct inode *inode, struct page **page, u64 off,
   	}
   	return ret;
   }
+
+/**
+ * ceph_fscrypt_prepare_readdir - simple __fscrypt_prepare_readdir() wrapper
+ * @dir: directory inode for readdir prep
+ *
+ * Simple wrapper around __fscrypt_prepare_readdir() that will mark directory as
+ * non-complete if this call results in having the directory unlocked.
+ *
+ * Returns:
+ *     1 - if directory was locked and key is now loaded (i.e. dir is unlocked)
+ *     0 - if directory is still locked
+ *   < 0 - if __fscrypt_prepare_readdir() fails
+ */
+int ceph_fscrypt_prepare_readdir(struct inode *dir)
+{
+	bool had_key = fscrypt_has_encryption_key(dir);
+	int err;
+
+	if (!IS_ENCRYPTED(dir))
+		return 0;
+
+	err = __fscrypt_prepare_readdir(dir);
+	if (err)
+		return err;
+	if (!had_key && fscrypt_has_encryption_key(dir)) {
+		/* directory just got unlocked, mark it as not complete */
+		ceph_dir_clear_complete(dir);
+		return 1;
+	}
+	return 0;
+}
diff --git a/fs/ceph/crypto.h b/fs/ceph/crypto.h
index c6ee993f4ec8..cad53ec916fd 100644
--- a/fs/ceph/crypto.h
+++ b/fs/ceph/crypto.h
@@ -154,6 +154,7 @@ int ceph_fscrypt_decrypt_extents(struct inode *inode, struct page **page, u64 of
   				 struct ceph_sparse_extent *map, u32 ext_cnt);
   int ceph_fscrypt_encrypt_pages(struct inode *inode, struct page **page, u64 off,
   				int len, gfp_t gfp);
+int ceph_fscrypt_prepare_readdir(struct inode *dir);
     static inline struct page *ceph_fscrypt_pagecache_page(struct page *page)
   {
@@ -254,6 +255,11 @@ static inline struct page *ceph_fscrypt_pagecache_page(struct page *page)
   {
   	return page;
   }
+
+static inline int ceph_fscrypt_prepare_readdir(struct inode *dir)
+{
+	return 0;
+}
   #endif /* CONFIG_FS_ENCRYPTION */
     static inline loff_t ceph_fscrypt_page_offset(struct page *page)
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index edc2bf0aab83..5829f27d058d 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -343,8 +343,8 @@ static int ceph_readdir(struct file *file, struct dir_context *ctx)
   		ctx->pos = 2;
   	}
   -	err = fscrypt_prepare_readdir(inode);
-	if (err)
+	err = ceph_fscrypt_prepare_readdir(inode);
+	if (err < 0)
   		return err;
     	spin_lock(&ci->i_ceph_lock);
@@ -784,8 +784,8 @@ static struct dentry *ceph_lookup(struct inode *dir, struct dentry *dentry,
   		return ERR_PTR(-ENAMETOOLONG);
     	if (IS_ENCRYPTED(dir)) {
-		err = __fscrypt_prepare_readdir(dir);
-		if (err)
+		err = ceph_fscrypt_prepare_readdir(dir);
+		if (err < 0)
   			return ERR_PTR(err);
   		if (!fscrypt_has_encryption_key(dir)) {
   			spin_lock(&dentry->d_lock);
diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c
index 9518ac8e407d..4becc9eada4b 100644
--- a/fs/ceph/mds_client.c
+++ b/fs/ceph/mds_client.c
@@ -2551,8 +2551,8 @@ static u8 *get_fscrypt_altname(const struct ceph_mds_request *req, u32 *plen)
   	if (!IS_ENCRYPTED(dir))
   		goto success;
   -	ret = __fscrypt_prepare_readdir(dir);
-	if (ret)
+	ret = ceph_fscrypt_prepare_readdir(dir);
+	if (ret < 0)
   		return ERR_PTR(ret);
     	/* No key? Just ignore it. */
@@ -2668,7 +2668,7 @@ char *ceph_mdsc_build_path(struct dentry *dentry, int *plen, u64 *pbase, int for
   			spin_unlock(&cur->d_lock);
   			parent = dget_parent(cur);
   -			ret = __fscrypt_prepare_readdir(d_inode(parent));
+			ret = ceph_fscrypt_prepare_readdir(d_inode(parent));
   			if (ret < 0) {
   				dput(parent);
   				dput(cur);
Hi Luis,

This version LGTM.

Just one nit. I think the following:

if (fscrypt_has_encryption_key(d_inode(parent))) {

is no needed any more.

We can just switch to:

if (ret) {

And also other places ?
No, that won't work, and I found that out the hard way :-)

If the directory is unlocked (i.e. the key is loaded), then the helper
function ceph_fscrypt_prepare_readdir() will return '0' because the status
hasn't change.  Thus, we still need to check that here.

Yeah, sounds reasonable!

LGTM and I will run the test later!

Thanks Luis.

- Xiubo


Cheers,




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux