On 2025-07-29 11:53:09, Amir Goldstein wrote: > On Mon, Jul 28, 2025 at 10:31 PM Andrey Albershteyn <aalbersh@xxxxxxxxxx> wrote: > > > > From: Andrey Albershteyn <aalbersh@xxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > [djwong: fix broken verity flag checks] > > Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> > > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx> > > --- > > 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
