[PATCH] ceph: set DCACHE_NOKEY_NAME in atomic open

[PATCH] ceph: set DCACHE_NOKEY_NAME in atomic open

[Jeff Layton]

Atomic open can act as a lookup if handed a dentry that is negative on
the MDS. Ensure that we set DCACHE_NOKEY_NAME on the dentry in
atomic_open, if we don't have the key for the parent. Otherwise, we can
end up validating the dentry inappropriately if someone later adds a
key.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/ceph/file.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Another patch for the fscrypt series.

A much less heavy-handed fix for generic/580 and generic/593. I'll
probably fold this into an earlier patch in the series since it appears
to be a straightforward bug.

diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index eb04dc8f1f93..5072570c2203 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -765,8 +765,14 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 	req->r_args.open.mask = cpu_to_le32(mask);
 	req->r_parent = dir;
 	ihold(dir);
-	if (IS_ENCRYPTED(dir))
+	if (IS_ENCRYPTED(dir)) {
 		set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
+		if (!fscrypt_has_encryption_key(dir)) {
+			spin_lock(&dentry->d_lock);
+			dentry->d_flags |= DCACHE_NOKEY_NAME;
+			spin_unlock(&dentry->d_lock);
+		}
+	}
 
 	if (flags & O_CREAT) {
 		struct ceph_file_layout lo;
-- 
2.35.1

Re: [PATCH] ceph: set DCACHE_NOKEY_NAME in atomic open

[Xiubo Li]

On 3/29/22 4:33 AM, Jeff Layton wrote:
> Atomic open can act as a lookup if handed a dentry that is negative on
> the MDS. Ensure that we set DCACHE_NOKEY_NAME on the dentry in
> atomic_open, if we don't have the key for the parent. Otherwise, we can
> end up validating the dentry inappropriately if someone later adds a
> key.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/file.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>
> Another patch for the fscrypt series.
>
> A much less heavy-handed fix for generic/580 and generic/593. I'll
> probably fold this into an earlier patch in the series since it appears
> to be a straightforward bug.
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index eb04dc8f1f93..5072570c2203 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -765,8 +765,14 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>   	req->r_args.open.mask = cpu_to_le32(mask);
>   	req->r_parent = dir;
>   	ihold(dir);
> -	if (IS_ENCRYPTED(dir))
> +	if (IS_ENCRYPTED(dir)) {
>   		set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> +		if (!fscrypt_has_encryption_key(dir)) {
> +			spin_lock(&dentry->d_lock);
> +			dentry->d_flags |= DCACHE_NOKEY_NAME;
> +			spin_unlock(&dentry->d_lock);
> +		}
> +	}
>   
>   	if (flags & O_CREAT) {
>   		struct ceph_file_layout lo;
Reviewed-by: Xiubo Li <xiubli@redhat.com>

Re: [PATCH] ceph: set DCACHE_NOKEY_NAME in atomic open

[Luís Henriques]

Jeff Layton <jlayton@kernel.org> writes:

> Atomic open can act as a lookup if handed a dentry that is negative on
> the MDS. Ensure that we set DCACHE_NOKEY_NAME on the dentry in
> atomic_open, if we don't have the key for the parent. Otherwise, we can
> end up validating the dentry inappropriately if someone later adds a
> key.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/file.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> Another patch for the fscrypt series.
>
> A much less heavy-handed fix for generic/580 and generic/593. I'll
> probably fold this into an earlier patch in the series since it appears
> to be a straightforward bug.

Ah!  This seems to be it, thanks Jeff.  One thing that may be worth doing
is to turn this pattern into an inline function, as it is repeated in a
few other places.  But anyway:

Reviewed-by: Luís Henriques <lhenriques@suse.de>

Cheers,
-- 
Luís

> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index eb04dc8f1f93..5072570c2203 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -765,8 +765,14 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
>  	req->r_args.open.mask = cpu_to_le32(mask);
>  	req->r_parent = dir;
>  	ihold(dir);
> -	if (IS_ENCRYPTED(dir))
> +	if (IS_ENCRYPTED(dir)) {
>  		set_bit(CEPH_MDS_R_FSCRYPT_FILE, &req->r_req_flags);
> +		if (!fscrypt_has_encryption_key(dir)) {
> +			spin_lock(&dentry->d_lock);
> +			dentry->d_flags |= DCACHE_NOKEY_NAME;
> +			spin_unlock(&dentry->d_lock);
> +		}
> +	}
>  
>  	if (flags & O_CREAT) {
>  		struct ceph_file_layout lo;
> -- 
>
> 2.35.1
>