On Tue, Dec 13, 2022 at 06:29:30PM +0100, Andrey Albershteyn wrote:
> fs-verity will read and attach metadata (not the tree itself) from
> a disk for those inodes which already have fs-verity enabled.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> ---
> fs/xfs/xfs_file.c | 8 ++++++++
> fs/xfs/xfs_super.c | 2 ++
> 2 files changed, 10 insertions(+)
>
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 242165580e682..5eadd9a37c50e 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -32,6 +32,7 @@
> #include <linux/mman.h>
> #include <linux/fadvise.h>
> #include <linux/mount.h>
> +#include <linux/fsverity.h>
>
> static const struct vm_operations_struct xfs_file_vm_ops;
>
> @@ -1170,9 +1171,16 @@ xfs_file_open(
> struct inode *inode,
> struct file *file)
> {
> + int error = 0;
> +
> if (xfs_is_shutdown(XFS_M(inode->i_sb)))
> return -EIO;
> file->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC | FMODE_BUF_WASYNC;
> +
> + error = fsverity_file_open(inode, file);
> + if (error)
> + return error;
This is a hot path, so shouldn't we elide the function call
altogether if verity is not enabled on the inode? i.e:
if (IS_VERITY(inode)) {
error = fsverity_file_open(inode, file);
if (error)
return error;
}
It doesn't really matter for a single file open, but when you're
opening a few million inodes every second the function call overhead
only to immediately return because IS_VERITY() is false adds up...
> return generic_file_open(inode, file);
> }
>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 8f1e9b9ed35d9..50c2c819ba940 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -45,6 +45,7 @@
> #include <linux/magic.h>
> #include <linux/fs_context.h>
> #include <linux/fs_parser.h>
> +#include <linux/fsverity.h>
>
> static const struct super_operations xfs_super_operations;
>
> @@ -647,6 +648,7 @@ xfs_fs_destroy_inode(
> ASSERT(!rwsem_is_locked(&inode->i_rwsem));
> XFS_STATS_INC(ip->i_mount, vn_rele);
> XFS_STATS_INC(ip->i_mount, vn_remove);
> + fsverity_cleanup_inode(inode);
Similarly, shouldn't this be:
if (fsverity_active(inode))
fsverity_cleanup_inode(inode);
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
