Re: [PATCH v2 0/2] API for exporting connectable file handles to userspace

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

 



On Tue, Oct 8, 2024 at 1:07 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Mon, 2024-10-07 at 17:26 +0200, Amir Goldstein wrote:
> > On Wed, Sep 25, 2024 at 11:14 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > >
> > > > open_by_handle_at(2) does not have AT_ flags argument, but also, I find
> > > > it more useful API that encoding a connectable file handle can mandate
> > > > the resolving of a connected fd, without having to opt-in for a
> > > > connected fd independently.
> > >
> > > This seems the best option to me too if this api is to be added.
> >
> > Thanks.
> >
> > Jeff, Chuck,
> >
> > Any thoughts on this?
> >
>
> Sorry for the delay. I think encoding the new flag into the fh itself
> is a reasonable approach.
>

Adding Jan.
Sorry I forgot to CC you on the patches, but struct file_handle is officially
a part of fanotify ABI, so your ACK is also needed on this change.

> I'm less thrilled with using bitfields for this, just because I have a
> general dislike of them, and they aren't implemented the same way on
> all arches. Would it break ABI if we just turned the handle_type int
> into two uint16_t's instead?

I think it will because this will not be backward compat on LE arch:

 struct file_handle {
        __u32 handle_bytes;
-       int handle_type;
+      __u16 handle_type;
+      __u16 handle_flags;
        /* file identifier */
        unsigned char f_handle[] __counted_by(handle_bytes);
 };

But I can also do without the bitfileds, maybe it's better this way.
See diff from v2:

diff --git a/fs/fhandle.c b/fs/fhandle.c
index 4ce4ffddec62..64d44fc61d43 100644
--- a/fs/fhandle.c
+++ b/fs/fhandle.c
@@ -87,9 +87,9 @@ static long do_sys_name_to_handle(const struct path *path,
                 * decoding connectable non-directory file handles.
                 */
                if (fh_flags & EXPORT_FH_CONNECTABLE) {
+                       handle->handle_type |= FILEID_IS_CONNECTABLE;
                        if (d_is_dir(path->dentry))
-                               fh_flags |= EXPORT_FH_DIR_ONLY;
-                       handle->handle_flags = fh_flags;
+                               fh_flags |= FILEID_IS_DIR;
                }
                retval = 0;
        }
@@ -352,7 +352,7 @@ static int handle_to_path(int mountdirfd, struct
file_handle __user *ufh,
                retval = -EINVAL;
                goto out_path;
        }
-       if (f_handle.handle_flags & ~EXPORT_FH_USER_FLAGS) {
+       if (!FILEID_USER_TYPE_IS_VALID(f_handle.handle_type)) {
                retval = -EINVAL;
                goto out_path;
        }
@@ -377,10 +377,14 @@ static int handle_to_path(int mountdirfd, struct
file_handle __user *ufh,
         * are decoding an fd with connected path, which is accessible from
         * the mount fd path.
         */
-       ctx.fh_flags |= f_handle.handle_flags;
-       if (ctx.fh_flags & EXPORT_FH_CONNECTABLE)
+       if (f_handle.handle_type & FILEID_IS_CONNECTABLE) {
+               ctx.fh_flags |= EXPORT_FH_CONNECTABLE;
                ctx.flags |= HANDLE_CHECK_SUBTREE;
-
+               if (f_handle.handle_type & FILEID_IS_DIR)
+                       ctx.fh_flags |= EXPORT_FH_DIR_ONLY;
+       }
+       /* Filesystem code should not be exposed to user flags */
+       handle->handle_type &= ~FILEID_USER_FLAGS_MASK;
        retval = do_handle_to_path(handle, path, &ctx);

 out_handle:
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 96b62e502f71..3e60bac74fa3 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -159,8 +159,17 @@ struct fid {
 #define EXPORT_FH_CONNECTABLE  0x1 /* Encode file handle with parent */
 #define EXPORT_FH_FID          0x2 /* File handle may be non-decodeable */
 #define EXPORT_FH_DIR_ONLY     0x4 /* Only decode file handle for a
directory */
-/* Flags allowed in encoded handle_flags that is exported to user */
-#define EXPORT_FH_USER_FLAGS   (EXPORT_FH_CONNECTABLE | EXPORT_FH_DIR_ONLY)
+
+/* Flags supported in encoded handle_type that is exported to user */
+#define FILEID_USER_FLAGS_MASK 0xffff0000
+#define FILEID_USER_FLAGS(type) ((type) & FILEID_USER_FLAGS_MASK)
+
+#define FILEID_IS_CONNECTABLE  0x10000
+#define FILEID_IS_DIR          0x40000
+#define FILEID_VALID_USER_FLAGS        (FILEID_IS_CONNECTABLE | FILEID_IS_DIR)
+
+#define FILEID_USER_TYPE_IS_VALID(type) \
+       (FILEID_USER_FLAGS(type) & ~FILEID_VALID_USER_FLAGS)

 /**
  * struct export_operations - for nfsd to communicate with file systems
diff --git a/include/linux/fs.h b/include/linux/fs.h
index cca7e575d1f8..6329fec40872 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1071,8 +1071,7 @@ struct file {

 struct file_handle {
        __u32 handle_bytes;
-       int handle_type:16;
-       int handle_flags:16;
+       int handle_type;
        /* file identifier */
        unsigned char f_handle[] __counted_by(handle_bytes);
 };





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux