| Message ID | 20200904160537.76663-16-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ceph+fscrypt: context, filename and symlink support | expand |
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
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.
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 --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 */
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(+)