diff mbox series

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

Message ID 20200904160537.76663-16-jlayton@kernel.org
State Superseded
Headers show
Series ceph+fscrypt: context, filename and symlink support | expand

Commit Message

Jeff Layton Sept. 4, 2020, 4:05 p.m. UTC
If we have an encrypted dentry, then we need to test whether a new key
might have been established or removed. Do that before we test anything
else about the dentry.

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

Comments

Eric Biggers Sept. 8, 2020, 5:12 a.m. UTC | #1
On Fri, Sep 04, 2020 at 12:05:34PM -0400, Jeff Layton wrote:
> If we have an encrypted dentry, then we need to test whether a new key
> might have been established or removed. Do that before we test anything
> else about the dentry.

A more accurate explanation would be:

"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."

> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/ceph/dir.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index b3f2741becdb..cc85933413b9 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1695,6 +1695,12 @@ 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);
>  
> +	if (IS_ENCRYPTED(dir)) {
> +		valid = fscrypt_d_revalidate(dentry, flags);
> +		if (valid <= 0)
> +			return valid;
> +	}

There's no need to check IS_ENCRYPTED(dir) here.

- Eric
Jeff Layton Sept. 9, 2020, 12:26 p.m. UTC | #2
On Mon, 2020-09-07 at 22:12 -0700, Eric Biggers wrote:
> On Fri, Sep 04, 2020 at 12:05:34PM -0400, Jeff Layton wrote:
> > If we have an encrypted dentry, then we need to test whether a new key
> > might have been established or removed. Do that before we test anything
> > else about the dentry.
> 
> A more accurate explanation would be:
> 
> "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."
> 

Can't a key also be removed (e.g. fscrypt lock /path/to/dir)?

Does that result in the dentries below that point being invalidated?

> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/ceph/dir.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index b3f2741becdb..cc85933413b9 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -1695,6 +1695,12 @@ 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);
> >  
> > +	if (IS_ENCRYPTED(dir)) {
> > +		valid = fscrypt_d_revalidate(dentry, flags);
> > +		if (valid <= 0)
> > +			return valid;
> > +	}
> 
> There's no need to check IS_ENCRYPTED(dir) here.
> 

Thanks, fixed in my tree.
Eric Biggers Sept. 9, 2020, 4:18 p.m. UTC | #3
On Wed, Sep 09, 2020 at 08:26:51AM -0400, Jeff Layton wrote:
> On Mon, 2020-09-07 at 22:12 -0700, Eric Biggers wrote:
> > On Fri, Sep 04, 2020 at 12:05:34PM -0400, Jeff Layton wrote:
> > > If we have an encrypted dentry, then we need to test whether a new key
> > > might have been established or removed. Do that before we test anything
> > > else about the dentry.
> > 
> > A more accurate explanation would be:
> > 
> > "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."
> > 
> 
> Can't a key also be removed (e.g. fscrypt lock /path/to/dir)?
> 
> Does that result in the dentries below that point being invalidated?

It results in the dentries (and inodes) being evicted, not invalidated.
See try_to_lock_encrypted_files() in fs/crypto/keyring.c.

So, fscrypt_d_revalidate() doesn't need to consider key removal.

- Eric
diff mbox series

Patch

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index b3f2741becdb..cc85933413b9 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -1695,6 +1695,12 @@  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);
 
+	if (IS_ENCRYPTED(dir)) {
+		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 */