linux-kernel - Re: [PATCH v6 6/6] fs: introduce file_getattr and file

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: <20250702-stagnation-dackel-294bb4cd9f3d@brauner>
Date: Wed, 2 Jul 2025 14:40:18 +0200
From: Christian Brauner <brauner@...nel.org>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Andrey Albershteyn <aalbersh@...hat.com>, 
	Amir Goldstein <amir73il@...il.com>, Arnd Bergmann <arnd@...db.de>, 
	Casey Schaufler <casey@...aufler-ca.com>, Jan Kara <jack@...e.cz>, 
	Pali Rohár <pali@...nel.org>, Paul Moore <paul@...l-moore.com>, linux-api@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, linux-xfs@...r.kernel.org, 
	selinux@...r.kernel.org, Andrey Albershteyn <aalbersh@...nel.org>
Subject: Re: [PATCH v6 6/6] fs: introduce file_getattr and file_setattr
 syscalls

> Er... "fsx_fileattr" is the struct that the system call uses?
> 
> That's a little confusing considering that xfs already has a
> xfs_fill_fsxattr function that actually fills a struct fileattr.
> That could be renamed xfs_fill_fileattr.
> 
> I dunno.  There's a part of me that would really rather that the
> file_getattr and file_setattr syscalls operate on a struct file_attr.

Agreed, I'm pretty sure I suggested this during an earlier review. Fits
in line with struct mount_attr and others. Fwiw, struct fileattr (the
kernel internal thing) should've really been struct file_kattr or struct
kernel_file_attr. This is a common pattern now:

struct mount_attr vs struct mount_kattr

struct clone_args vs struct kernel_clone_kargs

etc.

> 
> More whining/bikeshedding to come.
> 
> <snip stuff that looks ok to me>
> 
> <<well, I still dislike the CLASS(fd, fd)(fd) syntax...>>

Noted, and duly ignored...

> 
> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index 0098b0ce8ccb..0784f2033ba4 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -148,6 +148,24 @@ struct fsxattr {
> >  	unsigned char	fsx_pad[8];
> >  };
> >  
> > +/*
> > + * Variable size structure for file_[sg]et_attr().
> > + *
> > + * Note. This is alternative to the structure 'struct fileattr'/'struct fsxattr'.
> > + * As this structure is passed to/from userspace with its size, this can
> > + * be versioned based on the size.
> > + */
> > +struct fsx_fileattr {
> > +	__u32	fsx_xflags;	/* xflags field value (get/set) */
> 
> Should this to be __u64 from the start?  Seeing as (a) this struct is

Agreed. I changed that.