Re: [PATCH RFC 03/29] fs: add FS_XFLAG_VERITY for verity files

Andrey Albershteyn
2025-07-29T10:35:57Z

On 2025-07-29 11:53:09, Amir Goldstein wrote:
> On Mon, Jul 28, 2025 at 10:31 PM Andrey Albershteyn <aalbersh@redhat.com> wrote:
> >
> > From: Andrey Albershteyn <aalbersh@redhat.com>
> >
> > Add extended attribute FS_XFLAG_VERITY for inodes with fs-verity
> > enabled.
>
> Oh man! Please don't refer to this as an "extended attribute".
>
> I was quite surprised to see actually how many times the term
> "extended attribute" was used in commit messages in your series
> that Linus just merged including 4 such references in the Kernel-doc
> comments of security_inode_file_[sg]etattr(). :-/

You can comment on this, I'm fine with fixing these, I definitely
don't have all the context to know the best suitable terms.

>
> The terminology used in Documentation/filesystem/vfs.rst and fileattr.h
> are some permutations of "miscellaneous file flags and attributes".
> Not perfect, but less confusing than "extended attributes", which are
> famously known as something else.

Yeah sorry, it's very difficult to find out what is named how and
what is outdated.

I will update commit messages to "file flags".

>
> >
> > Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
> > [djwong: fix broken verity flag checks]
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> > ---
> > Documentation/filesystems/fsverity.rst | 8 ++++++++
> > fs/ioctl.c | 11 +++++++++++
> > include/uapi/linux/fs.h | 1 +
> > 3 files changed, 20 insertions(+)
> >
> > diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> > index dacdbc1149e6..33b588c32ed1 100644
> > --- a/Documentation/filesystems/fsverity.rst
> > +++ b/Documentation/filesystems/fsverity.rst
> > @@ -342,6 +342,14 @@ the file has fs-verity enabled. This can perform better than
> > FS_IOC_GETFLAGS and FS_IOC_MEASURE_VERITY because it doesn't require
> > opening the file, and opening verity files can be expensive.
> >
> > +FS_IOC_FSGETXATTR
> > +-----------------
> > +
> > +Since Linux v6.17, the FS_IOC_FSGETXATTR ioctl sets FS_XFLAG_VERITY (0x00020000)
> > +in the returned flags when the file has verity enabled. Note that this attribute
> > +cannot be set with FS_IOC_FSSETXATTR as enabling verity requires input
> > +parameters. See FS_IOC_ENABLE_VERITY.
> > +
> > .. _accessing_verity_files:
> >
> > Accessing verity files
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 69107a245b4c..6b94da2b93f5 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -480,6 +480,8 @@ void fileattr_fill_xflags(struct fileattr *fa, u32 xflags)
> > fa->flags |= FS_DAX_FL;
> > if (fa->fsx_xflags & FS_XFLAG_PROJINHERIT)
> > fa->flags |= FS_PROJINHERIT_FL;
> > + if (fa->fsx_xflags & FS_XFLAG_VERITY)
> > + fa->flags |= FS_VERITY_FL;
> > }
> > EXPORT_SYMBOL(fileattr_fill_xflags);
> >
> > @@ -510,6 +512,8 @@ void fileattr_fill_flags(struct fileattr *fa, u32 flags)
> > fa->fsx_xflags |= FS_XFLAG_DAX;
> > if (fa->flags & FS_PROJINHERIT_FL)
> > fa->fsx_xflags |= FS_XFLAG_PROJINHERIT;
> > + if (fa->flags & FS_VERITY_FL)
> > + fa->fsx_xflags |= FS_XFLAG_VERITY;
> > }
> > EXPORT_SYMBOL(fileattr_fill_flags);
> >
>
> I think you should add it to FS_COMMON_FL/FS_XFLAG_COMMON?
>
> And I guess also to FS_XFLAGS_MASK/FS_XFLAG_RDONLY_MASK
> after rebasing to master.
>
> > @@ -640,6 +644,13 @@ static int fileattr_set_prepare(struct inode *inode,
> > !(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)))
> > return -EINVAL;
> >
> > + /*
> > + * Verity cannot be changed through FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS.
> > + * See FS_IOC_ENABLE_VERITY.
> > + */
> > + if ((fa->fsx_xflags ^ old_ma->fsx_xflags) & FS_XFLAG_VERITY)
> > + return -EINVAL;
> > +
>
> I think that after rebase, if you add the flag to FS_XFLAG_RDONLY_MASK
> This check will fail, so can either remove this check and ignore user trying to
> set FS_XFLAG_VERITY, same as if user was trying to set FS_XFLAG_HASATTR.
> or, as I think Darrick may have suggested during review, we remove the masking
> of FS_XFLAG_RDONLY_MASK in the fill helpers and we make this check more
> generic here:
>
> + if ((fa->fsx_xflags ^ old_ma->fsx_xflags) & FS_XFLAG_RDONLY_MASK)
> + return -EINVAL;

Sure, will look into that in the next revision, I haven't used the
latest master, so this wasn't updated

>
> Thanks,
> Amir.
>

--
- Andrey