Messages in this thread | | | Subject | Re: [PATCH v3] ceph: mark directory as non-complete complete after loading key | From | Xiubo Li <> | Date | Wed, 23 Nov 2022 09:06:25 +0800 |
| |
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@suse.de> > --- > 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 ?
Thanks!
- Xiubo
>
| |