linux-kernel - Re: [PATCH RFC] fhandle: expose u64 mount id to name_to_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxiaRGypAB0v49FW8Se+=4e4to1FAg77sxLPCkO55KcuHQ@mail.gmail.com>
Date: Tue, 21 May 2024 08:04:09 +0300
From: Amir Goldstein <amir73il@...il.com>
To: Aleksa Sarai <cyphar@...har.com>
Cc: Jeff Layton <jlayton@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Chuck Lever <chuck.lever@...cle.com>, 
	Alexander Aring <alex.aring@...il.com>, linux-fsdevel@...r.kernel.org, 
	linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Christian Göttsche <cgoettsche@...tendoof.de>
Subject: Re: [PATCH RFC] fhandle: expose u64 mount id to name_to_handle_at(2)

On Tue, May 21, 2024 at 1:28 AM Aleksa Sarai <cyphar@...har.com> wrote:
>
> On 2024-05-20, Jeff Layton <jlayton@...nel.org> wrote:
> > On Mon, 2024-05-20 at 17:35 -0400, Aleksa Sarai wrote:
> > > Now that we have stabilised the unique 64-bit mount ID interface in
> > > statx, we can now provide a race-free way for name_to_handle_at(2) to
> > > provide a file handle and corresponding mount without needing to worry
> > > about racing with /proc/mountinfo parsing.

Both statx() and name_to_handle_at() support AT_EMPTY_PATH, so
there is a race-free way to get a file handle and unique mount id
for statmount().

Why do you mean /proc/mountinfo parsing?

> > >
> > > As with AT_HANDLE_FID, AT_HANDLE_UNIQUE_MNT_ID reuses a statx AT_* bit
> > > that doesn't make sense for name_to_handle_at(2).

Christian is probably regretting merging AT_HANDLE_FID now ;-)

Seriously, I would rearrange the AT_* flags namespace this way to
explicitly declare the overloaded per-syscall AT_* flags and possibly
prepare for the upcoming setxattrat(2) syscall [1].

[1] https://lore.kernel.org/linux-fsdevel/20240426162042.191916-1-cgoettsche@seltendoof.de/

The reason I would avoid overloading the AT_STATX_* flags is that
they describe a generic behavior that could potentially be relevant to
other syscalls in the future, e.g.:
renameat2(..., AT_RENAME_TEMPFILE | AT_FORCE_SYNC);

But then again, I don't understand why you need to extend name_to_handle_at()
at all for your purpose...

Thanks,
Amir.

--- a/include/uapi/linux/fcntl.h
+++ b/include/uapi/linux/fcntl.h
[...]
+
+#define AT_PRIVATE_FLAGS       0x2ff   /* Per-syscall flags mask.  */
+
+/* Common flags for *at() syscalls */
 #define AT_SYMLINK_NOFOLLOW    0x100   /* Do not follow symbolic links.  */
-#define AT_EACCESS             0x200   /* Test access permitted for
-                                           effective IDs, not real IDs.  */
-#define AT_REMOVEDIR           0x200   /* Remove directory instead of
-                                           unlinking file.  */
 #define AT_SYMLINK_FOLLOW      0x400   /* Follow symbolic links.  */
 #define AT_NO_AUTOMOUNT                0x800   /* Suppress terminal
automount traversal */
 #define AT_EMPTY_PATH          0x1000  /* Allow empty relative pathname */

+/* Flags for statx(2) */
 #define AT_STATX_SYNC_TYPE     0x6000  /* Type of synchronisation
required from statx() */
 #define AT_STATX_SYNC_AS_STAT  0x0000  /* - Do whatever stat() does */
 #define AT_STATX_FORCE_SYNC    0x2000  /* - Force the attributes to
be sync'd with the server */
[...]

 #define AT_RECURSIVE           0x8000  /* Apply to the entire subtree */

-/* Flags for name_to_handle_at(2). We reuse AT_ flag space to save bits... */
-#define AT_HANDLE_FID          AT_REMOVEDIR    /* file handle is needed to
+/* Flags for name_to_handle_at(2) */
+#define AT_HANDLE_FID          0x200   /* file handle is needed to
                                        compare object identity and may not
                                        be usable to open_by_handle_at(2) */
+/* Flags for faccessat(2) */
+#define AT_EACCESS             0x200   /* Test access permitted for
+                                           effective IDs, not real IDs.  */
+/* Flags for unlinkat(2) */
+#define AT_REMOVEDIR           0x200   /* Remove directory instead of
+                                           unlinking file.  */
+
+/* Flags for renameat2(2) (should match legacy RENAME_* flags) */
+#define AT_RENAME_NOREPLACE    0x001   /* Don't overwrite target */
+#define AT_RENAME_EXCHANGE     0x002   /* Exchange source and dest */
+#define AT_RENAME_WHITEOUT     0x004   /* Whiteout source */
+#define AT_RENAME_TEMPFILE     0x008   /* Source file is O_TMPFILE */
+
+/* Flags for setxattrat(2) (should match legacy XATTR_* flags) */
+#define AT_XATTR_CREATE                0x001   /* set value, fail if
attr already exists */
+#define AT_XATTR_REPLACE       0x002   /* set value, fail if attr
does not exist */
+