linux-kernel - Re: [GIT PULL] vfs dio

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] [day] [month] [year] [list]

Message-ID: <20250122-aalen-knabbern-3a6d2f4b9c92@brauner>
Date: Wed, 22 Jan 2025 14:50:42 +0100
From: Christian Brauner <brauner@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] vfs dio

On Mon, Jan 20, 2025 at 11:50:09AM -0800, Linus Torvalds wrote:
> On Mon, 20 Jan 2025 at 11:38, Christian Brauner <brauner@...nel.org> wrote:
> >
> > It is heavily used nowadays though because there's a few
> > additional bits in there that don't require calling into filesystems
> > but are heavily used.
> 
> By "heavily used" you mean "there is probably a 1:1000 ratio between
> statx:stat in reality".
> 
> Those "few additional bits" are all very specialized. No normal
> application cares about things like "mount ID" or subvolume data. I
> can't imagine what other fields you think are so important.

Sorry, I wasn't talking about new fields I was talking about actual bits
that statx has.

#define STATX_ATTR_COMPRESSED		0x00000004 /* [I] File is compressed by the fs */
#define STATX_ATTR_IMMUTABLE		0x00000010 /* [I] File is marked immutable */
#define STATX_ATTR_APPEND		0x00000020 /* [I] File is append-only */
#define STATX_ATTR_NODUMP		0x00000040 /* [I] File is not to be dumped */
#define STATX_ATTR_ENCRYPTED		0x00000800 /* [I] File requires key to decrypt in fs */
#define STATX_ATTR_AUTOMOUNT		0x00001000 /* Dir: Automount trigger */
#define STATX_ATTR_MOUNT_ROOT		0x00002000 /* Root of a mount */
#define STATX_ATTR_VERITY		0x00100000 /* [I] Verity protected file */
#define STATX_ATTR_DAX			0x00200000 /* File is currently in DAX state */
#define STATX_ATTR_WRITE_ATOMIC		0x00400000 /* File supports atomic write operations */

I think that such an attribute field could've been added to struct stat
itself because afaict there are two unused fields in there currently:

struct stat {
	unsigned long	st_dev;		/* Device.  */
	unsigned long	st_ino;		/* File serial number.  */
	unsigned int	st_mode;	/* File mode.  */
	unsigned int	st_nlink;	/* Link count.  */
	unsigned int	st_uid;		/* User ID of the file's owner.  */
	unsigned int	st_gid;		/* Group ID of the file's group. */
	[...]
	unsigned int	__unused4;
	unsigned int	__unused5;
};

I happily concede that stat() will exceed the usage of statx() for most
application that e.g., just want to do the basic mode, uid, gid thing.
So compared to that statx() usage is probably not that important.

And nothing would stop us from adding a statx2()...

int statx(int dfd, const char *path, unsigned int flags, unsigned int mask
          struct statx *st, size_t st_size)

#define STATX_SIZE_VER0 // compatible with struct stat

and then only the stat portion is filled in. This could allow glibc to
use statx() for everything without perf impact especially when paired
with allowing NULL with AT_EMPTY_PATH.