diff mbox series

[v4,05/25] fs: add FS_XFLAG_VERITY for verity files

Message ID 20240212165821.1901300-6-aalbersh@redhat.com (mailing list archive)
State Superseded, archived
Headers show
Series fs-verity support for XFS | expand

Commit Message

Andrey Albershteyn Feb. 12, 2024, 4:58 p.m. UTC
Add extended attribute FS_XFLAG_VERITY for inodes with fs-verity
enabled.

Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com>
---
 Documentation/filesystems/fsverity.rst | 12 ++++++++++++
 fs/ioctl.c                             | 11 +++++++++++
 include/uapi/linux/fs.h                |  1 +
 3 files changed, 24 insertions(+)

Comments

Eric Biggers Feb. 23, 2024, 4:23 a.m. UTC | #1
On Mon, Feb 12, 2024 at 05:58:02PM +0100, Andrey Albershteyn wrote:
> +FS_IOC_FSGETXATTR
> +-----------------
> +
> +Since Linux v6.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
> +files. The attribute can be observed via lsattr.
> +
> +    [root@vm:~]# lsattr /mnt/test/foo
> +    --------------------V- /mnt/test/foo
> +
> +Note that this attribute cannot be set with FS_IOC_FSSETXATTR as enabling verity
> +requires input parameters. See FS_IOC_ENABLE_VERITY.

The lsattr example is irrelevant and misleading because lsattr uses
FS_IOC_GETFLAGS, not FS_IOC_FSGETXATTR.

Also, I know that you titled the subsection "FS_IOC_FSGETXATTR", but the text
itself should make it super clear that FS_XFLAG_VERITY is only for
FS_IOC_FSGETXATTR, not FS_IOC_GETFLAGS.

> diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> index 48ad69f7722e..6e63ea832d4f 100644
> --- a/include/uapi/linux/fs.h
> +++ b/include/uapi/linux/fs.h
> @@ -140,6 +140,7 @@ struct fsxattr {
>  #define FS_XFLAG_FILESTREAM	0x00004000	/* use filestream allocator */
>  #define FS_XFLAG_DAX		0x00008000	/* use DAX for IO */
>  #define FS_XFLAG_COWEXTSIZE	0x00010000	/* CoW extent size allocator hint */
> +#define FS_XFLAG_VERITY		0x00020000	/* fs-verity sealed inode */

There's currently nowhere in the documentation or code that uses the phrase
"fs-verity sealed inode".  It's instead called a verity file, or a file that has
fs-verity enabled.  We should try to avoid inconsistent terminology.

- Eric
Andrey Albershteyn Feb. 23, 2024, 12:55 p.m. UTC | #2
On 2024-02-22 20:23:04, Eric Biggers wrote:
> On Mon, Feb 12, 2024 at 05:58:02PM +0100, Andrey Albershteyn wrote:
> > +FS_IOC_FSGETXATTR
> > +-----------------
> > +
> > +Since Linux v6.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
> > +files. The attribute can be observed via lsattr.
> > +
> > +    [root@vm:~]# lsattr /mnt/test/foo
> > +    --------------------V- /mnt/test/foo
> > +
> > +Note that this attribute cannot be set with FS_IOC_FSSETXATTR as enabling verity
> > +requires input parameters. See FS_IOC_ENABLE_VERITY.
> 
> The lsattr example is irrelevant and misleading because lsattr uses
> FS_IOC_GETFLAGS, not FS_IOC_FSGETXATTR.
> 
> Also, I know that you titled the subsection "FS_IOC_FSGETXATTR", but the text
> itself should make it super clear that FS_XFLAG_VERITY is only for
> FS_IOC_FSGETXATTR, not FS_IOC_GETFLAGS.

Sure, I will remove the example. Would something like this be clear
enough?

    FS_IOC_FSGETXATTR
    -----------------

    Since Linux v6.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
    files. This attribute can be checked with FS_IOC_FSGETXATTR ioctl. Note that
    this attribute cannot be set with FS_IOC_FSSETXATTR as enabling verity requires
    input parameters. See FS_IOC_ENABLE_VERITY.

> > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
> > index 48ad69f7722e..6e63ea832d4f 100644
> > --- a/include/uapi/linux/fs.h
> > +++ b/include/uapi/linux/fs.h
> > @@ -140,6 +140,7 @@ struct fsxattr {
> >  #define FS_XFLAG_FILESTREAM	0x00004000	/* use filestream allocator */
> >  #define FS_XFLAG_DAX		0x00008000	/* use DAX for IO */
> >  #define FS_XFLAG_COWEXTSIZE	0x00010000	/* CoW extent size allocator hint */
> > +#define FS_XFLAG_VERITY		0x00020000	/* fs-verity sealed inode */
> 
> There's currently nowhere in the documentation or code that uses the phrase
> "fs-verity sealed inode".  It's instead called a verity file, or a file that has
> fs-verity enabled.  We should try to avoid inconsistent terminology.

Oops, missed this one. Thanks!
Eric Biggers Feb. 23, 2024, 5:59 p.m. UTC | #3
On Fri, Feb 23, 2024 at 01:55:21PM +0100, Andrey Albershteyn wrote:
> On 2024-02-22 20:23:04, Eric Biggers wrote:
> > On Mon, Feb 12, 2024 at 05:58:02PM +0100, Andrey Albershteyn wrote:
> > > +FS_IOC_FSGETXATTR
> > > +-----------------
> > > +
> > > +Since Linux v6.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
> > > +files. The attribute can be observed via lsattr.
> > > +
> > > +    [root@vm:~]# lsattr /mnt/test/foo
> > > +    --------------------V- /mnt/test/foo
> > > +
> > > +Note that this attribute cannot be set with FS_IOC_FSSETXATTR as enabling verity
> > > +requires input parameters. See FS_IOC_ENABLE_VERITY.
> > 
> > The lsattr example is irrelevant and misleading because lsattr uses
> > FS_IOC_GETFLAGS, not FS_IOC_FSGETXATTR.
> > 
> > Also, I know that you titled the subsection "FS_IOC_FSGETXATTR", but the text
> > itself should make it super clear that FS_XFLAG_VERITY is only for
> > FS_IOC_FSGETXATTR, not FS_IOC_GETFLAGS.
> 
> Sure, I will remove the example. Would something like this be clear
> enough?
> 
>     FS_IOC_FSGETXATTR
>     -----------------
> 
>     Since Linux v6.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
>     files. This attribute can be checked with FS_IOC_FSGETXATTR ioctl. Note that
>     this attribute cannot be set with FS_IOC_FSSETXATTR as enabling verity requires
>     input parameters. See FS_IOC_ENABLE_VERITY.

It's better, but I'd probably put FS_IOC_FSGETXATTR in the first sentence.
Like: Since Linux v6.9, the FS_IOC_FSGETXATTR ioctl sets FS_XFLAG_VERITY
(0x00020000) in the returned flags when the file has verity enabled.

- Eric
diff mbox series

Patch

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 13e4b18e5dbb..19e59e87999e 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -326,6 +326,18 @@  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.9, FS_XFLAG_VERITY (0x00020000) file attribute is set for verity
+files. The attribute can be observed via lsattr.
+
+    [root@vm:~]# lsattr /mnt/test/foo
+    --------------------V- /mnt/test/foo
+
+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 76cf22ac97d7..38c00e47c069 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -481,6 +481,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);
 
@@ -511,6 +513,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);
 
@@ -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;
+
 	/* Extent size hints of zero turn off the flags. */
 	if (fa->fsx_extsize == 0)
 		fa->fsx_xflags &= ~(FS_XFLAG_EXTSIZE | FS_XFLAG_EXTSZINHERIT);
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 48ad69f7722e..6e63ea832d4f 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -140,6 +140,7 @@  struct fsxattr {
 #define FS_XFLAG_FILESTREAM	0x00004000	/* use filestream allocator */
 #define FS_XFLAG_DAX		0x00008000	/* use DAX for IO */
 #define FS_XFLAG_COWEXTSIZE	0x00010000	/* CoW extent size allocator hint */
+#define FS_XFLAG_VERITY		0x00020000	/* fs-verity sealed inode */
 #define FS_XFLAG_HASATTR	0x80000000	/* no DIFLAG for this	*/
 
 /* the read-only stuff doesn't really belong here, but any other place is