On Thu, Mar 07, 2024 at 01:39:11PM -0800, Darrick J. Wong wrote: > On Mon, Mar 04, 2024 at 02:35:48PM -0800, Eric Biggers wrote: > > On Mon, Mar 04, 2024 at 08:10:28PM +0100, Andrey Albershteyn wrote: > > > @@ -641,6 +645,13 @@ static int fileattr_set_prepare(struct inode *inode, > > > !(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) > > > return -EINVAL; > > > > > > + /* > > > + * Verity cannot be set through FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS. > > > + * See FS_IOC_ENABLE_VERITY > > > + */ > > > + if (fa->fsx_xflags & FS_XFLAG_VERITY) > > > + return -EINVAL; > > > > This makes FS_IOC_SETFLAGS and FS_IOC_FSSETXATTR start failing on files that > > already have verity enabled. > > > > An error should only be returned when the new flags contain verity and the old > > flags don't. > > What if the old flags have it and the new ones don't? Is that supposed > to disable fsverity? Is removal of the verity information not supported? > > I'm guessing that removal isn't supposed to happen, in which case the > above check ought to be: > > if (!!IS_VERITY(inode) != !!(fa->fsx_xflags & FS_XFLAG_VERITY)) > return -EINVAL; > > Right? Yeah, good catch. We need to prevent disabling the flag too. How about: if ((fa->flags ^ old_ma->flags) & FS_VERITY_FL) return -EINVAL; That would be consistent with how changes to other flags such as FS_APPEND_FL and FS_IMMUTABLE_FL are detected earlier in the function. - Eric
