Re: [RFC][PATCH 2/4] exportfs: add explicit flag to request non-decodeable file handles

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 27, 2023 at 6:00 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Tue, 2023-04-25 at 16:01 +0300, Amir Goldstein wrote:
> > So far, all callers of exportfs_encode_inode_fh(), except for fsnotify's
> > show_mark_fhandle(), check that filesystem can decode file handles, but
> > we would like to add more callers that do not require a file handle that
> > can be decoded.
> >
> > Introduce a flag to explicitly request a file handle that may not to be
> > decoded later and a wrapper exportfs_encode_fid() that sets this flag
> > and convert show_mark_fhandle() to use the new wrapper.
> >
> > This will be used to allow adding fanotify support to filesystems that
> > do not support NFS export.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> >  Documentation/filesystems/nfs/exporting.rst |  4 ++--
> >  fs/exportfs/expfs.c                         | 18 ++++++++++++++++--
> >  fs/notify/fanotify/fanotify.c               |  4 ++--
> >  fs/notify/fdinfo.c                          |  2 +-
> >  include/linux/exportfs.h                    | 12 +++++++++++-
> >  5 files changed, 32 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/filesystems/nfs/exporting.rst b/Documentation/filesystems/nfs/exporting.rst
> > index 0e98edd353b5..3d97b8d8f735 100644
> > --- a/Documentation/filesystems/nfs/exporting.rst
> > +++ b/Documentation/filesystems/nfs/exporting.rst
> > @@ -122,8 +122,8 @@ are exportable by setting the s_export_op field in the struct
> >  super_block.  This field must point to a "struct export_operations"
> >  struct which has the following members:
> >
> > - encode_fh  (optional)
> > -    Takes a dentry and creates a filehandle fragment which can later be used
> > +  encode_fh (optional)
> > +    Takes a dentry and creates a filehandle fragment which may later be used
> >      to find or create a dentry for the same object.  The default
> >      implementation creates a filehandle fragment that encodes a 32bit inode
> >      and generation number for the inode encoded, and if necessary the
> > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
> > index bf1b4925fedd..1b35dda5bdda 100644
> > --- a/fs/exportfs/expfs.c
> > +++ b/fs/exportfs/expfs.c
> > @@ -381,11 +381,25 @@ static int export_encode_fh(struct inode *inode, struct fid *fid,
> >       return type;
> >  }
> >
> > +/**
> > + * exportfs_encode_inode_fh - encode a file handle from inode
> > + * @inode:   the object to encode
> > + * @fid:     where to store the file handle fragment
> > + * @max_len: maximum length to store there
> > + * @flags:   properties of the requrested file handle
> > + */
> >  int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
> > -                          int *max_len, struct inode *parent)
> > +                          int *max_len, struct inode *parent, int flags)
> >  {
> >       const struct export_operations *nop = inode->i_sb->s_export_op;
> >
> > +     /*
> > +      * If a decodeable file handle was requested, we need to make sure that
> > +      * filesystem can decode file handles.
> > +      */
> > +     if (nop && !(flags & EXPORT_FH_FID) && !nop->fh_to_dentry)
> > +             return -EOPNOTSUPP;
> > +
>
> If you're moving this check into this function, then it might be good to
> remove the same check from the callers that are doing this check now.
>

There are three types of callers:
1. nfsd and fanotify check nop->fh_to_dentry at export/setup time
    so I cannot remove those checks
2. in do_sys_name_to_handle() I could have removed the duplicate
    check but then -EFAULT/-EINVAL could be returned instead of
    -EOPNOTSUPP and I did not want to make that visible API change
3. show_mark_fhandle() does not check anything (*)

(*) The reason that show_mark_fhandle() exists originally is to aid CRIU
to restore inotify/fanotify watches on container restore.
Since CRIU cannot really do that without decoding file handles
it is somewhat questionable that show_mark_fhandle() does not check for
->fh_to_dentry and it looks like an oversight, but it has been like that for
too long to change this behavior IMO.

> >       if (nop && nop->encode_fh)
> >               return nop->encode_fh(inode, fid->raw, max_len, parent);
> >
> > @@ -416,7 +430,7 @@ int exportfs_encode_fh(struct dentry *dentry, struct fid *fid, int *max_len,
> >               parent = p->d_inode;
> >       }
> >
> > -     error = exportfs_encode_inode_fh(inode, fid, max_len, parent);
> > +     error = exportfs_encode_inode_fh(inode, fid, max_len, parent, flags);
> >       dput(p);
> >
> >       return error;
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 29bdd99b29fa..d1a49f5b6e6d 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -380,7 +380,7 @@ static int fanotify_encode_fh_len(struct inode *inode)
> >       if (!inode)
> >               return 0;
> >
> > -     exportfs_encode_inode_fh(inode, NULL, &dwords, NULL);
> > +     exportfs_encode_inode_fh(inode, NULL, &dwords, NULL, 0);
> >       fh_len = dwords << 2;
> >
> >       /*
> > @@ -443,7 +443,7 @@ static int fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
> >       }
> >
> >       dwords = fh_len >> 2;
> > -     type = exportfs_encode_inode_fh(inode, buf, &dwords, NULL);
> > +     type = exportfs_encode_inode_fh(inode, buf, &dwords, NULL, 0);
> >       err = -EINVAL;
> >       if (!type || type == FILEID_INVALID || fh_len != dwords << 2)
> >               goto out_err;
> > diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
> > index 55081ae3a6ec..5c430736ec12 100644
> > --- a/fs/notify/fdinfo.c
> > +++ b/fs/notify/fdinfo.c
> > @@ -50,7 +50,7 @@ static void show_mark_fhandle(struct seq_file *m, struct inode *inode)
> >       f.handle.handle_bytes = sizeof(f.pad);
> >       size = f.handle.handle_bytes >> 2;
> >
> > -     ret = exportfs_encode_inode_fh(inode, (struct fid *)f.handle.f_handle, &size, NULL);
> > +     ret = exportfs_encode_fid(inode, (struct fid *)f.handle.f_handle, &size);
> >       if ((ret == FILEID_INVALID) || (ret < 0)) {
> >               WARN_ONCE(1, "Can't encode file handler for inotify: %d\n", ret);
> >               return;
> > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> > index 2b1048238170..635e89e1dae7 100644
> > --- a/include/linux/exportfs.h
> > +++ b/include/linux/exportfs.h
> > @@ -136,6 +136,7 @@ struct fid {
> >  };
> >
> >  #define EXPORT_FH_CONNECTABLE        0x1
> > +#define EXPORT_FH_FID                0x2
>
> Please add comments about what these flags are intended to indicate.
>

OK.

Thanks,
Amir.




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux