Re: [PATCH v5 05/24] fs: add FS_XFLAG_VERITY for verity files

Eric Biggers
2024-03-07T22:06:33Z

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