| Message ID | 20240304191046.157464-7-aalbersh@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fs-verity support for XFS | expand |
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. - Eric
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? --D > - Eric >
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
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst index 13e4b18e5dbb..887cdaf162a9 100644 --- a/Documentation/filesystems/fsverity.rst +++ b/Documentation/filesystems/fsverity.rst @@ -326,6 +326,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.9, 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 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..b1d0e1169bc3 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 enabled */ #define FS_XFLAG_HASATTR 0x80000000 /* no DIFLAG for this */ /* the read-only stuff doesn't really belong here, but any other place is
Add extended attribute FS_XFLAG_VERITY for inodes with fs-verity enabled. Signed-off-by: Andrey Albershteyn <aalbersh@redhat.com> --- Documentation/filesystems/fsverity.rst | 8 ++++++++ fs/ioctl.c | 11 +++++++++++ include/uapi/linux/fs.h | 1 + 3 files changed, 20 insertions(+)