linux-kernel - Re: [PATCH v6 5/6] fs: prepare for extending 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: <kxl5mwdql2f7i6mlvgitmquhci6oab5dluihc22x5w7y3cko4h@fig54f3dyidj>
Date: Tue, 1 Jul 2025 15:06:25 +0200
From: Jan Kara <jack@...e.cz>
To: Andrey Albershteyn <aalbersh@...hat.com>
Cc: Amir Goldstein <amir73il@...il.com>, Arnd Bergmann <arnd@...db.de>, 
	Casey Schaufler <casey@...aufler-ca.com>, Christian Brauner <brauner@...nel.org>, 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 5/6] fs: prepare for extending file_get/setattr()

On Mon 30-06-25 18:20:15, Andrey Albershteyn wrote:
> From: Amir Goldstein <amir73il@...il.com>
> 
> We intend to add support for more xflags to selective filesystems and
> We cannot rely on copy_struct_from_user() to detect this extension.
> 
> In preparation of extending the API, do not allow setting xflags unknown
> by this kernel version.
> 
> Also do not pass the read-only flags and read-only field fsx_nextents to
> filesystem.
> 
> These changes should not affect existing chattr programs that use the
> ioctl to get fsxattr before setting the new values.
> 
> Link: https://lore.kernel.org/linux-fsdevel/20250216164029.20673-4-pali@kernel.org/
> Cc: Pali Rohár <pali@...nel.org>
> Cc: Andrey Albershteyn <aalbersh@...hat.com>
> Signed-off-by: Amir Goldstein <amir73il@...il.com>
> Signed-off-by: Andrey Albershteyn <aalbersh@...nel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

I'd just note that:

> @@ -118,11 +119,16 @@ static int copy_fsxattr_from_user(struct fileattr *fa,
>  				  struct fsxattr __user *ufa)
>  {
>  	struct fsxattr xfa;
> +	__u32 mask = FS_XFLAGS_MASK;
>  
>  	if (copy_from_user(&xfa, ufa, sizeof(xfa)))
>  		return -EFAULT;
>  
> +	if (xfa.fsx_xflags & ~mask)
> +		return -EINVAL;
> +
>  	fileattr_fill_xflags(fa, xfa.fsx_xflags);
> +	fa->fsx_xflags &= ~FS_XFLAG_RDONLY_MASK;

This means that the two flags in FS_XFLAG_RDONLY_MASK cannot easily become
writeable in the future due to this. I think it is a sensible compromise
but I wanted to mention it.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR