Re: [PATCH v9 01/17] fs: Move file_kattr initialization to callers

Darrick J. Wong
2026-04-22T23:38:24Z

On Wed, Apr 22, 2026 at 07:29:55PM -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
>
> fileattr_fill_xflags() and fileattr_fill_flags() memset the
> entire file_kattr struct before populating select fields, so
> callers cannot pre-set fields in fa->fsx_xflags without having
> their values clobbered. Darrick Wong noted that a function
> named "fill_xflags" touching more than xflags forces callers
> to know implementation details beyond its apparent scope.
>
> Drop the memset from both fill functions and initialize at the
> entry points instead: ioctl_setflags(), ioctl_fssetxattr(),
> the file_setattr() syscall, and xfs_ioc_fsgetxattra() now
> declare fa with an aggregate initializer. ioctl_getflags(),
> ioctl_fsgetxattr(), and the file_getattr() syscall already
> aggregate-initialize fa to pass flags_valid/fsx_valid hints
> into vfs_fileattr_get().
>
> Subsequent patches rely on this so that ->fileattr_get()
> handlers can set case-sensitivity flags (FS_XFLAG_CASEFOLD,
> FS_XFLAG_CASENONPRESERVING) in fa->fsx_xflags before the fill
> functions run.
>
> Suggested-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Heh, I never did review this one so
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> fs/file_attr.c | 12 ++++--------
> fs/xfs/xfs_ioctl.c | 2 +-
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/fs/file_attr.c b/fs/file_attr.c
> index da983e105d70..f429da66a317 100644
> --- a/fs/file_attr.c
> +++ b/fs/file_attr.c
> @@ -15,12 +15,10 @@
> * @fa: fileattr pointer
> * @xflags: FS_XFLAG_* flags
> *
> - * Set ->fsx_xflags, ->fsx_valid and ->flags (translated xflags). All
> - * other fields are zeroed.
> + * Set ->fsx_xflags, ->fsx_valid and ->flags (translated xflags).
> */
> void fileattr_fill_xflags(struct file_kattr *fa, u32 xflags)
> {
> - memset(fa, 0, sizeof(*fa));
> fa->fsx_valid = true;
> fa->fsx_xflags = xflags;
> if (fa->fsx_xflags & FS_XFLAG_IMMUTABLE)
> @@ -48,11 +46,9 @@ EXPORT_SYMBOL(fileattr_fill_xflags);
> * @flags: FS_*_FL flags
> *
> * Set ->flags, ->flags_valid and ->fsx_xflags (translated flags).
> - * All other fields are zeroed.
> */
> void fileattr_fill_flags(struct file_kattr *fa, u32 flags)
> {
> - memset(fa, 0, sizeof(*fa));
> fa->flags_valid = true;
> fa->flags = flags;
> if (fa->flags & FS_SYNC_FL)
> @@ -325,7 +321,7 @@ int ioctl_setflags(struct file *file, unsigned int __user *argp)
> {
> struct mnt_idmap *idmap = file_mnt_idmap(file);
> struct dentry *dentry = file->f_path.dentry;
> - struct file_kattr fa;
> + struct file_kattr fa = {};
> unsigned int flags;
> int err;
>
> @@ -357,7 +353,7 @@ int ioctl_fssetxattr(struct file *file, void __user *argp)
> {
> struct mnt_idmap *idmap = file_mnt_idmap(file);
> struct dentry *dentry = file->f_path.dentry;
> - struct file_kattr fa;
> + struct file_kattr fa = {};
> int err;
>
> err = copy_fsxattr_from_user(&fa, argp);
> @@ -431,7 +427,7 @@ SYSCALL_DEFINE5(file_setattr, int, dfd, const char __user *, filename,
> struct path filepath __free(path_put) = {};
> unsigned int lookup_flags = 0;
> struct file_attr fattr;
> - struct file_kattr fa;
> + struct file_kattr fa = {};
> int error;
>
> BUILD_BUG_ON(sizeof(struct file_attr) < FILE_ATTR_SIZE_VER0);
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 46e234863644..ed9b4846c05f 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -517,7 +517,7 @@ xfs_ioc_fsgetxattra(
> xfs_inode_t *ip,
> void __user *arg)
> {
> - struct file_kattr fa;
> + struct file_kattr fa = {};
>
> xfs_ilock(ip, XFS_ILOCK_SHARED);
> xfs_fill_fsxattr(ip, XFS_ATTR_FORK, &fa);
>
> --
> 2.53.0
>
>