Re: [PATCH v7 03/43] fscrypt: add a __fscrypt_file_open helper

[Daniel Vacek <neelx@suse.com>]

On Tue, 2 Jun 2026 at 04:34, Eric Biggers <ebiggers@kernel.org> wrote:
> On Wed, May 13, 2026 at 10:52:37AM +0200, Daniel Vacek wrote:
> > From: Josef Bacik <josef@toxicpanda.com>
> >
> > We have fscrypt_file_open() which is meant to be called on files being
> > opened so that their key is loaded when we start reading data from them.
> >
> > However for btrfs send we are opening the inode directly without a filp,
> > so we need a different helper to make sure we can load the fscrypt
> > context for the inode before reading its contents.
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Daniel Vacek <neelx@suse.com>
> > ---
> >
> > No changes in v7.
> > v6 changes:
> >  * Adapted to fscrypt changes since the last two years.
> > v5: https://lore.kernel.org/linux-btrfs/4a372419c3fe6ad425e1b124c342a054e9d6db23.1706116485.git.josef@toxicpanda.com/
> > ---
> >  fs/crypto/hooks.c       | 38 ++++++++++++++++++++++++++++++++------
> >  include/linux/fscrypt.h |  8 ++++++++
> >  2 files changed, 40 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> > index a7a8a3f581a0..3142cf106bde 100644
> > --- a/fs/crypto/hooks.c
> > +++ b/fs/crypto/hooks.c
> > @@ -9,6 +9,37 @@
> >
> >  #include "fscrypt_private.h"
> >
> > +/**
> > + * __fscrypt_file_open() - prepare for filesystem-internal access to a
> > + *                      possibly-encrypted regular file
> > + * @dir: the inode for the directory via which the file is being accessed
> > + * @inode: the inode being "opened"
> > + *
> > + * This is like fscrypt_file_open(), but instead of taking the 'struct file'
> > + * being opened it takes the parent directory explicitly.  This is intended for
> > + * use cases such as "send/receive" which involve the filesystem accessing file
> > + * contents without setting up a 'struct file'.
> > + *
> > + * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
> > + */
> > +int __fscrypt_file_open(struct inode *dir, struct inode *inode)
> > +{
> > +     int err;
> > +
> > +     err = fscrypt_require_key(inode);
> > +     if (err)
> > +             return err;
> > +
> > +     if (!fscrypt_has_permitted_context(dir, inode)) {
> > +             fscrypt_warn(inode,
> > +                          "Inconsistent encryption context (parent directory: %llu)",
> > +                          dir->i_ino);
> > +             return -EPERM;
> > +     }
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(__fscrypt_file_open);
> > +
> >  /**
> >   * fscrypt_file_open() - prepare to open a possibly-encrypted regular file
> >   * @inode: the inode being opened
> > @@ -60,12 +91,7 @@ int fscrypt_file_open(struct inode *inode, struct file *filp)
> >       rcu_read_unlock();
> >
> >       dentry_parent = dget_parent(dentry);
> > -     if (!fscrypt_has_permitted_context(d_inode(dentry_parent), inode)) {
> > -             fscrypt_warn(inode,
> > -                          "Inconsistent encryption context (parent directory: %llu)",
> > -                          d_inode(dentry_parent)->i_ino);
> > -             err = -EPERM;
> > -     }
> > +     err = __fscrypt_file_open(d_inode(dentry_parent), inode);
> >       dput(dentry_parent);
> >       return err;
> >  }
>
> This change makes fscrypt_file_open() execute an unnecessary extra
> fscrypt_require_key().  Could we just leave fscrypt_file_open() as-is?

Yes, I'm aware of that. Well, it's static inline and gated only for
encrypted inodes. But yeah, I'll refactor it a bit further.

--nX

> - Eric