| Message ID | 20221121180004.8038-1-lhenriques@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ceph: make sure directories aren't complete after setting crypt context | expand |
On 22/11/2022 02:00, 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@suse.de> > --- > Hi Xiubo! > > I've added the __fscrypt_prepare_readdir() wrapper as you suggested, but I > had to change it slightly because we also need to handle the error cases. > > 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/dir.c | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c > index edc2bf0aab83..2cac7e3ab352 100644 > --- a/fs/ceph/dir.c > +++ b/fs/ceph/dir.c > @@ -763,6 +763,27 @@ static bool is_root_ceph_dentry(struct inode *inode, struct dentry *dentry) > strncmp(dentry->d_name.name, ".ceph", 5) == 0; > } > > +/* > + * Simple wrapper around __fscrypt_prepare_readdir() that will return: > + * > + * - '1' if directory was locked and key is now loaded (i.e. dir is unlocked), > + * - '0' if directory is still locked, or > + * - an error (< 0) if __fscrypt_prepare_readdir() fails. > + */ > +static int ceph_fscrypt_prepare_readdir(struct inode *dir) > +{ > + bool had_key = fscrypt_has_encryption_key(dir); > + int err; > + > + err = __fscrypt_prepare_readdir(dir); > + if (err) > + return err; > + /* is directory now unlocked? */ > + if (!had_key && fscrypt_has_encryption_key(dir)) > + return 1; > + return 0; > +} > + > /* > * Look up a single dir entry. If there is a lookup intent, inform > * the MDS so that it gets our 'caps wanted' value in a single op. > @@ -784,10 +805,14 @@ 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)) { > + if (err) { > + /* directory just got unlocked */ > + __ceph_dir_clear_complete(ceph_inode(dir)); Luis, Could we just move this into ceph_fscrypt_prepare_readdir() ? IMO we should always clear the complete flag always whenever the key is first loaded. > + } else { > + /* no encryption key */ I think you also need to fix all the other places, which are calling the __fscrypt_prepare_readdir() or fscrypt_prepare_readdir() in ceph. Because we don't know the ceph_lookup() will always be the first caller to trigger __fscrypt_prepare_readdir() for a dir. Thanks! - Xiubo > spin_lock(&dentry->d_lock); > dentry->d_flags |= DCACHE_NOKEY_NAME; > spin_unlock(&dentry->d_lock); >
diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c index edc2bf0aab83..2cac7e3ab352 100644 --- a/fs/ceph/dir.c +++ b/fs/ceph/dir.c @@ -763,6 +763,27 @@ static bool is_root_ceph_dentry(struct inode *inode, struct dentry *dentry) strncmp(dentry->d_name.name, ".ceph", 5) == 0; } +/* + * Simple wrapper around __fscrypt_prepare_readdir() that will return: + * + * - '1' if directory was locked and key is now loaded (i.e. dir is unlocked), + * - '0' if directory is still locked, or + * - an error (< 0) if __fscrypt_prepare_readdir() fails. + */ +static int ceph_fscrypt_prepare_readdir(struct inode *dir) +{ + bool had_key = fscrypt_has_encryption_key(dir); + int err; + + err = __fscrypt_prepare_readdir(dir); + if (err) + return err; + /* is directory now unlocked? */ + if (!had_key && fscrypt_has_encryption_key(dir)) + return 1; + return 0; +} + /* * Look up a single dir entry. If there is a lookup intent, inform * the MDS so that it gets our 'caps wanted' value in a single op. @@ -784,10 +805,14 @@ 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)) { + if (err) { + /* directory just got unlocked */ + __ceph_dir_clear_complete(ceph_inode(dir)); + } else { + /* no encryption key */ spin_lock(&dentry->d_lock); dentry->d_flags |= DCACHE_NOKEY_NAME; spin_unlock(&dentry->d_lock);
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@suse.de> --- Hi Xiubo! I've added the __fscrypt_prepare_readdir() wrapper as you suggested, but I had to change it slightly because we also need to handle the error cases. 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/dir.c | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-)