Re: [PATCH v18 01/15] xfs: introduce fsverity on-disk changes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] xfs: FS_IOC_SETFLAGS fails with EOPNOTSUPP on verity files in xfs_fileattr_set()
--

commit 28c38f60096ab4965845dc0881e604defd26203f
Author: Andrey Albershteyn <aalbersh@kernel.org>

xfs: introduce fsverity on-disk changes

This commit introduces the on-disk XFS_DIFLAG2_VERITY inode flag and the
XFS_SB_FEAT_RO_COMPAT_VERITY superblock feature bit for fs-verity support.
It also wires up inode verification and flag conversions between on-disk
diflags2, extended attributes, and VFS inode flags.

[ ... ]

> diff --git a/fs/xfs/libxfs/xfs_inode_util.c b/fs/xfs/libxfs/xfs_inode_util.c
> index f486b23a9b523..a717b19558afa 100644
> --- a/fs/xfs/libxfs/xfs_inode_util.c
> +++ b/fs/xfs/libxfs/xfs_inode_util.c
> @@ -73,7 +73,8 @@ xfs_flags2diflags2(
>  	uint64_t		di_flags2 =
>  		(ip->i_diflags2 & (XFS_DIFLAG2_REFLINK |
>  				   XFS_DIFLAG2_BIGTIME |
> -				   XFS_DIFLAG2_NREXT64));
> +				   XFS_DIFLAG2_NREXT64 |
> +				   XFS_DIFLAG2_VERITY));
>  
>  	if (xflags & FS_XFLAG_DAX)
>  		di_flags2 |= XFS_DIFLAG2_DAX;
> @@ -125,6 +126,8 @@ xfs_ip2xflags(
>  			flags |= FS_XFLAG_DAX;
>  		if (ip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE)
>  			flags |= FS_XFLAG_COWEXTSIZE;
> +		if (ip->i_diflags2 & XFS_DIFLAG2_VERITY)
> +			flags |= FS_XFLAG_VERITY;

[Severity: Medium]
When FS_XFLAG_VERITY is returned by xfs_ip2xflags(), will a read-modify-write
cycle using FS_IOC_GETFLAGS and FS_IOC_SETFLAGS (such as chattr toggling
FS_IMMUTABLE_FL or FS_NOATIME_FL) on a verity-enabled inode fail with
-EOPNOTSUPP?

During FS_IOC_GETFLAGS on an inode with XFS_DIFLAG2_VERITY set,
vfs_fileattr_get() -> xfs_fileattr_get() -> xfs_fill_fsxattr() ->
fileattr_fill_xflags() translates FS_XFLAG_VERITY into FS_VERITY_FL in
fa->flags.

When userspace modifies another flag and passes the flags back via
FS_IOC_SETFLAGS, ioctl_setflags() calls fileattr_fill_flags(), which leaves
fa->fsx_valid as false while preserving FS_VERITY_FL in fa->flags. In
fs/xfs/xfs_ioctl.c:xfs_fileattr_set():

	if (!fa->fsx_valid) {
		unsigned int allowed = FS_IMMUTABLE_FL | FS_APPEND_FL |
				       FS_NOATIME_FL | FS_NODUMP_FL |
				       FS_SYNC_FL | FS_DAX_FL |
				       FS_PROJINHERIT_FL;

		if (xfs_has_asciici(mp))
			allowed |= FS_CASEFOLD_FL;

		if (fa->flags & ~allowed)
			return -EOPNOTSUPP;
	}

Should xfs_fileattr_set() allow FS_VERITY_FL in allowed when
ip->i_diflags2 & XFS_DIFLAG2_VERITY is set, similar to how FS_CASEFOLD_FL is
handled right above it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102144147.2393369-1-aalbersh@kernel.org?part=1