| Message ID | 20221122145920.17025-1-lhenriques@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ceph: mark directory as non-complete complete after loading key | expand |
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 >
Xiubo Li <xiubli@redhat.com> 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@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 ? 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. Cheers,
On 23/11/2022 17:59, Luís Henriques wrote: > Xiubo Li <xiubli@redhat.com> 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@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 ? > 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,
Hi Luis, BTW, which branch are you using to generate this patch ? Could you rebase to the testing branch ? Thanks - Xiubo 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); >
Xiubo Li <xiubli@redhat.com> writes: > Hi Luis, > > BTW, which branch are you using to generate this patch ? > > Could you rebase to the testing branch ? Hmmm, I think I've used the wip-fscrypt branch. OK, I'll resend this after running a few tests just to make sure. Cheers,
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);
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(-)