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 <[email protected]>
---
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));
+ } else {
+ /* no encryption key */
spin_lock(&dentry->d_lock);
dentry->d_flags |= DCACHE_NOKEY_NAME;
spin_unlock(&dentry->d_lock);
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 <[email protected]>
> ---
> 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);
>