Re: [PATCH 05/36] fscrypt: uninline and export fscrypt_require_key

[Jeff Layton <jlayton@kernel.org>]

On Sun, 2021-12-12 at 13:03 -0800, Eric Biggers wrote:
> On Sun, Dec 12, 2021 at 03:38:29PM -0500, Jeff Layton wrote:
> > On Sun, 2021-12-12 at 11:56 -0800, Eric Biggers wrote:
> > > On Fri, Dec 10, 2021 at 03:40:20PM -0500, Jeff Layton wrote:
> > > > On Fri, 2021-12-10 at 11:46 -0800, Eric Biggers wrote:
> > > > > On Thu, Dec 09, 2021 at 10:36:16AM -0500, Jeff Layton wrote:
> > > > > > ceph_atomic_open needs to be able to call this.
> > > > > > 
> > > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > > ---
> > > > > >  fs/crypto/fscrypt_private.h | 26 --------------------------
> > > > > >  fs/crypto/keysetup.c        | 27 +++++++++++++++++++++++++++
> > > > > >  include/linux/fscrypt.h     |  5 +++++
> > > > > >  3 files changed, 32 insertions(+), 26 deletions(-)
> > > > > 
> > > > > What is the use case for this, more precisely?  I've been trying to keep
> > > > > filesystems using helper functions like fscrypt_prepare_*() and
> > > > > fscrypt_file_open() rather than setting up encryption keys directly, which is a
> > > > > bit too low-level to be doing outside of fs/crypto/.
> > > > > 
> > > > > Perhaps fscrypt_file_open() is what you're looking for here?
> > > > 
> > > > That doesn't really help because we don't have the inode for the file
> > > > yet at the point where we need the key.
> > > > 
> > > > atomic_open basically does a lookup+open. You give it a directory inode
> > > > and a dentry, and it issues an open request by filename. If it gets back
> > > > ENOENT then we know that the thing is a negative dentry.
> > > > 
> > > > In the lookup path, I used __fscrypt_prepare_readdir. This situation is
> > > > a bit similar so I might be able to use that instead. OTOH, that doesn't
> > > > fail when you don't have the key, and if you don't, there's not a lot of
> > > > point in going any further here.
> > > 
> > > So you're requiring the key on a directory to do a lookup in that directory?
> > > Normally that's not required, as files can be looked up by no-key name.  Why is
> > > the atomic_open case different? 
> > > 
> > > The file inode's key is needed to open it, of
> > > course, but the directory inode's key shouldn't be needed.  In practice you'll
> > > tend to have the key for both or neither inode, but that's not guaranteed.
> > > 
> > 
> > We're issuing an open request to the server without looking up the inode
> > first. In order to do that open request, we need to encode a filename
> > into the request, and to do that we need the encryption key.
> 
> But how is it different from a regular lookup?  Those try to set up the
> directory's key, but if it's unavailable, the name being looked up is treated as
> a no-key name.  Take a look at fscrypt_prepare_lookup().
> 

Ok. After looking at this some more, I think you're right. I don't
really need this call in atomic_open at all. I'll plan to just drop this
patch from the series.

Thanks!
-- 
Jeff Layton <jlayton@kernel.org>