diff mbox series

[RFC,v4,15/17] ceph: make d_revalidate call fscrypt revalidator for encrypted dentries

Message ID 20210120182847.644850-16-jlayton@kernel.org (mailing list archive)
State New, archived
Headers show
Series ceph+fscrypt: context, filename and symlink support | expand

Commit Message

Jeff Layton Jan. 20, 2021, 6:28 p.m. UTC
If we have a dentry which represents a no-key name, then we need to test
whether the parent directory's encryption key has since been added.  Do
that before we test anything else about the dentry.

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

Comments

Luis Henriques Feb. 1, 2021, 5:18 p.m. UTC | #1
Jeff Layton <jlayton@kernel.org> writes:

> If we have a dentry which represents a no-key name, then we need to test
> whether the parent directory's encryption key has since been added.  Do
> that before we test anything else about the dentry.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/dir.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 236c381ab6bd..cb7ff91a243a 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1726,6 +1726,10 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
>  	dout("d_revalidate %p '%pd' inode %p offset 0x%llx\n", dentry,
>  	     dentry, inode, ceph_dentry(dentry)->offset);
>  
> +	valid = fscrypt_d_revalidate(dentry, flags);
> +	if (valid <= 0)
> +		return valid;
> +

This one took me a while to figure out, but eventually got there.
Initially I was seeing this error:

crypt: ceph: 1 inode(s) still busy after removing key with identifier f019f4a1c5d5665675218f89fccfa3c7, including ino 1099511627791

and, when umounting the filesystem I would get the warning in
fs/dcache.c:1623.

Anyway, the patch below should fix it.

Unfortunately I didn't had a lot of time to look into the -experimental
branch yet.  On my TODO list for the next few days.

Cheers,
Jeff Layton Feb. 1, 2021, 6:41 p.m. UTC | #2
On Mon, 2021-02-01 at 17:18 +0000, Luis Henriques wrote:
> Jeff Layton <jlayton@kernel.org> writes:
> 
> > If we have a dentry which represents a no-key name, then we need to test
> > whether the parent directory's encryption key has since been added.  Do
> > that before we test anything else about the dentry.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/dir.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index 236c381ab6bd..cb7ff91a243a 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -1726,6 +1726,10 @@ static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
> >  	dout("d_revalidate %p '%pd' inode %p offset 0x%llx\n", dentry,
> >  	     dentry, inode, ceph_dentry(dentry)->offset);
> >  
> > 
> > +	valid = fscrypt_d_revalidate(dentry, flags);
> > +	if (valid <= 0)
> > +		return valid;
> > +
> 
> This one took me a while to figure out, but eventually got there.
> Initially I was seeing this error:
> 
> crypt: ceph: 1 inode(s) still busy after removing key with identifier f019f4a1c5d5665675218f89fccfa3c7, including ino 1099511627791
> 
> and, when umounting the filesystem I would get the warning in
> fs/dcache.c:1623.
> 
> Anyway, the patch below should fix it.
> 
> Unfortunately I didn't had a lot of time to look into the -experimental
> branch yet.  On my TODO list for the next few days.
> 
> Cheers,

Well spotted! I think the better fix though is to just move the
fscrypt_d_revalidate call up before the point where we take the parent
reference. I'll fix that up in my tree. Thanks for tracking that down!
diff mbox series

Patch

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 236c381ab6bd..cb7ff91a243a 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1726,6 +1726,10 @@  static int ceph_d_revalidate(struct dentry *dentry, unsigned int flags)
 	dout("d_revalidate %p '%pd' inode %p offset 0x%llx\n", dentry,
 	     dentry, inode, ceph_dentry(dentry)->offset);
 
+	valid = fscrypt_d_revalidate(dentry, flags);
+	if (valid <= 0)
+		return valid;
+
 	mdsc = ceph_sb_to_client(dir->i_sb)->mdsc;
 
 	/* always trust cached snapped dentries, snapdir dentry */