Re: [PATCH RFC 22/29] xfs: add fs-verity support

Darrick J. Wong
2025-07-29T23:05:37Z

On Mon, Jul 28, 2025 at 10:30:26PM +0200, Andrey Albershteyn wrote:
> Add integration with fs-verity. XFS stores fs-verity descriptor in
> the extended file attributes and the Merkle tree is store in data fork
> beyond EOF.
>
> The descriptor is stored under "vdesc" extended attribute.
>
> The Merkle tree reading/writing is done through iomap interface. The
> data itself are at offset 1 << 53 in the inode's page cache. When XFS
> reads from this region iomap doesn't call into fsverity to verify it
> against Merkle tree. For data, verification is done on BIO completion.
>
> When fs-verity is enabled on an inode, the XFS_IVERITY_CONSTRUCTION
> flag is set meaning that the Merkle tree is being build. The
> initialization ends with storing of verity descriptor and setting
> inode on-disk flag (XFS_DIFLAG2_VERITY).
>
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

/me wonders if this is really SoB me since it's rather different from
the last time I even touched this patch. It also means I can't RVB it.
;)

> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> fs/xfs/Makefile | 1 +
> fs/xfs/libxfs/xfs_da_format.h | 4 +
> fs/xfs/xfs_bmap_util.c | 7 +
> fs/xfs/xfs_fsverity.c | 311 ++++++++++++++++++++++++++++++++++++++++++
> fs/xfs/xfs_fsverity.h | 28 ++++
> fs/xfs/xfs_inode.h | 6 +
> fs/xfs/xfs_super.c | 20 +++
> 7 files changed, 377 insertions(+)
>
> diff --git a/fs/xfs/Makefile b/fs/xfs/Makefile
> index 5bf501cf8271..ad66439db7bf 100644
> --- a/fs/xfs/Makefile
> +++ b/fs/xfs/Makefile
> @@ -147,6 +147,7 @@ xfs-$(CONFIG_XFS_POSIX_ACL) += xfs_acl.o
> xfs-$(CONFIG_SYSCTL) += xfs_sysctl.o
> xfs-$(CONFIG_COMPAT) += xfs_ioctl32.o
> xfs-$(CONFIG_EXPORTFS_BLOCK_OPS) += xfs_pnfs.o
> +xfs-$(CONFIG_FS_VERITY) += xfs_fsverity.o
>
> # notify failure
> ifeq ($(CONFIG_MEMORY_FAILURE),y)
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index e5274be2fe9c..b17fdbbb48aa 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -909,4 +909,8 @@ struct xfs_parent_rec {
> __be32 p_gen;
> } __packed;
>
> +/* fs-verity ondisk xattr name used for the fsverity descriptor */
> +#define XFS_VERITY_DESCRIPTOR_NAME "vdesc"
> +#define XFS_VERITY_DESCRIPTOR_NAME_LEN (sizeof(XFS_VERITY_DESCRIPTOR_NAME) - 1)
> +
> #endif /* __XFS_DA_FORMAT_H__ */
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 06ca11731e43..af1933129647 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -31,6 +31,7 @@
> #include "xfs_rtbitmap.h"
> #include "xfs_rtgroup.h"
> #include "xfs_zone_alloc.h"
> +#include <linux/fsverity.h>
>
> /* Kernel only BMAP related definitions and functions */
>
> @@ -553,6 +554,12 @@ xfs_can_free_eofblocks(
> if (last_fsb <= end_fsb)
> return false;
>
> + /*
> + * Nothing to clean on fsverity inodes as they are read-only
> + */
> + if (IS_VERITY(VFS_I(ip)))
> + return false;
> +
> /*
> * Check if there is an post-EOF extent to free. If there are any
> * delalloc blocks attached to the inode (data fork delalloc
> diff --git a/fs/xfs/xfs_fsverity.c b/fs/xfs/xfs_fsverity.c
> new file mode 100644
> index 000000000000..ec7dea4289d5
> --- /dev/null
> +++ b/fs/xfs/xfs_fsverity.c
> @@ -0,0 +1,311 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023 Red Hat, Inc.
> + */
> +#include "xfs.h"
> +#include "xfs_shared.h"
> +#include "xfs_format.h"
> +#include "xfs_da_format.h"
> +#include "xfs_da_btree.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_inode.h"
> +#include "xfs_log_format.h"
> +#include "xfs_attr.h"
> +#include "xfs_bmap_util.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans.h"
> +#include "xfs_attr_leaf.h"
> +#include "xfs_trace.h"
> +#include "xfs_quota.h"
> +#include "xfs_ag.h"
> +#include "xfs_fsverity.h"
> +#include "xfs_iomap.h"
> +#include <linux/fsverity.h>
> +#include <linux/pagemap.h>
> +
> +/*
> + * Initialize an args structure to load or store the fsverity descriptor.
> + * Caller must ensure @args is zeroed except for value and valuelen.
> + */
> +static inline void
> +xfs_fsverity_init_vdesc_args(
> + struct xfs_inode *ip,
> + struct xfs_da_args *args)
> +{
> + args->geo = ip->i_mount->m_attr_geo;
> + args->whichfork = XFS_ATTR_FORK;
> + args->attr_filter = XFS_ATTR_VERITY;
> + args->op_flags = XFS_DA_OP_OKNOENT;
> + args->dp = ip;
> + args->owner = ip->i_ino;
> + args->name = XFS_VERITY_DESCRIPTOR_NAME;
> + args->namelen = XFS_VERITY_DESCRIPTOR_NAME_LEN;
> + xfs_attr_sethash(args);
> +}
> +
> +/* Delete the verity descriptor. */
> +static int
> +xfs_fsverity_delete_descriptor(
> + struct xfs_inode *ip)
> +{
> + struct xfs_da_args args = { };
> +
> + xfs_fsverity_init_vdesc_args(ip, &args);
> + return xfs_attr_set(&args, XFS_ATTRUPDATE_REMOVE, false);
> +}
> +
> +/* Retrieve the verity descriptor. */
> +static int
> +xfs_fsverity_get_descriptor(
> + struct inode *inode,
> + void *buf,
> + size_t buf_size)
> +{
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_da_args args = {
> + .value = buf,
> + .valuelen = buf_size,
> + };
> + int error = 0;
> +
> + /*
> + * The fact that (returned attribute size) == (provided buf_size) is
> + * checked by xfs_attr_copy_value() (returns -ERANGE). No descriptor
> + * is treated as a short read so that common fsverity code will
> + * complain.
> + */
> + xfs_fsverity_init_vdesc_args(ip, &args);
> + error = xfs_attr_get(&args);
> + if (error == -ENOATTR)
> + return 0;
> + if (error)
> + return error;
> +
> + return args.valuelen;
> +}
> +
> +/* Try to remove all the fsverity metadata after a failed enablement. */
> +static int
> +xfs_fsverity_delete_metadata(
> + struct xfs_inode *ip)
> +{
> + struct xfs_trans *tp;
> + struct xfs_mount *mp = ip->i_mount;
> + int error;
> +
> + error = xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, 0, 0, 0, &tp);
> + if (error) {
> + ASSERT(xfs_is_shutdown(mp));
> + return error;
> + }
> +
> + xfs_ilock(ip, XFS_ILOCK_EXCL);
> + xfs_trans_ijoin(tp, ip, 0);
> +
> + /*
> + * As only merkle tree is getting removed, no need to change inode size
> + */
> + error = xfs_itruncate_extents(&tp, ip, XFS_DATA_FORK, XFS_ISIZE(ip));
> + if (error)
> + goto err_cancel;
> +
> + error = xfs_trans_commit(tp);
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> + if (error)
> + return error;
> +
> + error = xfs_fsverity_delete_descriptor(ip);
> + return error != -ENOATTR ? error : 0;

Do you need to re-truncate the file to EOF to clear out the post-eof
blocks?

> +
> +err_cancel:
> + xfs_trans_cancel(tp);
> + return error;
> +}
> +
> +
> +/* Prepare to enable fsverity by clearing old metadata. */
> +static int
> +xfs_fsverity_begin_enable(
> + struct file *filp)
> +{
> + struct inode *inode = file_inode(filp);
> + struct xfs_inode *ip = XFS_I(inode);
> + int error;
> +
> + xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
> +
> + if (IS_DAX(inode))
> + return -EINVAL;
> +
> + if (inode->i_size > XFS_FSVERITY_MTREE_OFFSET)
> + return -EFBIG;
> +
> + if (xfs_iflags_test_and_set(ip, XFS_VERITY_CONSTRUCTION))
> + return -EBUSY;

Hm. Do we actually flush the pagecache to disk in the _begin_enable
path? /me looked briefly but didn't see anything.

> +
> + error = xfs_qm_dqattach(ip);
> + if (error)
> + return error;
> +
> + return xfs_fsverity_delete_metadata(ip);
> +}
> +
> +/* Complete (or fail) the process of enabling fsverity. */
> +static int
> +xfs_fsverity_end_enable(
> + struct file *filp,
> + const void *desc,
> + size_t desc_size,
> + u64 merkle_tree_size)
> +{
> + struct xfs_da_args args = {
> + .value = (void *)desc,
> + .valuelen = desc_size,
> + };
> + struct inode *inode = file_inode(filp);
> + struct xfs_inode *ip = XFS_I(inode);
> + struct xfs_mount *mp = ip->i_mount;
> + struct xfs_trans *tp;
> + int error = 0;
> +
> + xfs_assert_ilocked(ip, XFS_IOLOCK_EXCL);
> +
> + /* fs-verity failed, just cleanup */
> + if (desc == NULL)
> + goto out;
> +
> + xfs_fsverity_init_vdesc_args(ip, &args);
> + error = xfs_attr_set(&args, XFS_ATTRUPDATE_UPSERT, false);
> + if (error)
> + goto out;
> +
> + /*
> + * Wait for Merkel tree get written to disk before setting on-disk inode
> + * flag and clering XFS_VERITY_CONSTRUCTION

s/Merkel/Merkle/

s/clering/clearing/ , sorry if that's my typo

> + */
> + error = filemap_write_and_wait(inode->i_mapping);
> + if (error)
> + return error;
> +
> + /* Set fsverity inode flag */
> + error = xfs_trans_alloc_inode(ip, &M_RES(mp)->tr_ichange,
> + 0, 0, false, &tp);
> + if (error)
> + goto out;
> +
> + /*
> + * Ensure that we've persisted the verity information before we enable
> + * it on the inode and tell the caller we have sealed the inode.
> + */
> + ip->i_diflags2 |= XFS_DIFLAG2_VERITY;
> +
> + xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> + xfs_trans_set_sync(tp);
> +
> + error = xfs_trans_commit(tp);
> + xfs_iunlock(ip, XFS_ILOCK_EXCL);
> +
> + if (!error)
> + inode->i_flags |= S_VERITY;
> +
> +out:
> + if (error) {
> + int error2;
> +
> + error2 = xfs_fsverity_delete_metadata(ip);
> + if (error2)
> + xfs_alert(ip->i_mount,
> +"ino 0x%llx failed to clean up new fsverity metadata, err %d",
> + ip->i_ino, error2);
> + }
> +
> + xfs_iflags_clear(ip, XFS_VERITY_CONSTRUCTION);
> + return error;
> +}
> +
> +static void
> +xfs_fsverity_adjust_read(
> + struct ioregion *region)
> +{
> + u8 log_blocksize;
> + unsigned int block_size;
> + u64 tree_size;
> + u64 position = region->pos & XFS_FSVERITY_MTREE_MASK;
> +
> + fsverity_merkle_tree_geometry(region->inode, &log_blocksize,
> + &block_size, &tree_size);
> +
> + if (position + region->length < tree_size)
> + return;
> +
> + region->length = tree_size - position;
> +}
> +
> +/* Retrieve a merkle tree block. */
> +static struct page *
> +xfs_fsverity_read_merkle(
> + struct inode *inode,
> + pgoff_t index,
> + unsigned long num_ra_pages)
> +{
> + u64 position = (index << PAGE_SHIFT) | XFS_FSVERITY_MTREE_OFFSET;

Is this shift safe on 32-bit where pgoff_t is unsigned long?
I don't think it is....?

> + struct ioregion region = {
> + .inode = inode,
> + .pos = position,
> + .length = PAGE_SIZE,
> + .ops = &xfs_read_iomap_ops,
> + };
> + int error;
> + struct folio *folio;
> +
> + /*
> + * As region->length is PAGE_SIZE we have to adjust the length for the
> + * end of the tree. The case when tree blocks size are smaller then
> + * PAGE_SIZE.

Hrm. I wonder if we want to try to create large folios for merkle
trees?

Since the comment references PAGE_SIZE, what happens if this is an 8k
fsblock XFS filesystem where folios will always be at least 8k?

> + */
> + xfs_fsverity_adjust_read(&region);
> +
> + folio = iomap_read_region(&region);
> + if (IS_ERR(folio))
> + return ERR_PTR(-EIO);
> +
> + /* Wait for buffered read to finish */
> + error = folio_wait_locked_killable(folio);
> + if (error)
> + return ERR_PTR(error);
> + if (IS_ERR(folio) || !folio_test_uptodate(folio))
> + return ERR_PTR(-EFSCORRUPTED);
> +
> + return folio_file_page(folio, 0);
> +}
> +
> +/* Write a merkle tree block. */
> +static int
> +xfs_fsverity_write_merkle(
> + struct inode *inode,
> + const void *buf,
> + u64 pos,
> + unsigned int size)
> +{
> + struct ioregion region = {
> + .inode = inode,
> + .pos = pos | XFS_FSVERITY_MTREE_OFFSET,
> + .buf = buf,
> + .length = size,
> + .ops = &xfs_buffered_write_iomap_ops,
> + };
> +
> + if (region.pos + region.length > inode->i_sb->s_maxbytes)
> + return -EFBIG;
> +
> + return iomap_write_region(&region);
> +}
> +
> +const struct fsverity_operations xfs_fsverity_ops = {
> + .begin_enable_verity = xfs_fsverity_begin_enable,
> + .end_enable_verity = xfs_fsverity_end_enable,
> + .get_verity_descriptor = xfs_fsverity_get_descriptor,
> + .read_merkle_tree_page = xfs_fsverity_read_merkle,
> + .write_merkle_tree_block = xfs_fsverity_write_merkle,
> +};
> diff --git a/fs/xfs/xfs_fsverity.h b/fs/xfs/xfs_fsverity.h
> new file mode 100644
> index 000000000000..e063b7288dc0
> --- /dev/null
> +++ b/fs/xfs/xfs_fsverity.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 Red Hat, Inc.
> + */
> +#ifndef __XFS_FSVERITY_H__
> +#define __XFS_FSVERITY_H__
> +
> +/* Merkle tree location in page cache. We take memory region from the inode's
> + * address space for Merkle tree.
> + *
> + * At maximum of 8 levels with 128 hashes per block (32 bytes SHA-256) maximum

Aren't sha512 hashes allowed too? In which case it would be 64 bytes
per hash?

I suppose that doesn't really matter since... an 8EB file on a 1k
fsblock filesystem with 1k merkle tree blocks will do this:

# xfs_db /dev/sda -c "btheight -b 1024 -n $(( (2**63 - 1) / 1024 ))
64:64:0:merkle -w min"
64:64:0:merkle: worst case per 1024-byte block: 8 records (leaf) / 8
keyptrs (node)
level 0: 9007199254740991 records, 1125899906842624 blocks
level 1: 1125899906842624 records, 140737488355328 blocks
level 2: 140737488355328 records, 17592186044416 blocks
level 3: 17592186044416 records, 2199023255552 blocks
level 4: 2199023255552 records, 274877906944 blocks
level 5: 274877906944 records, 34359738368 blocks
level 6: 34359738368 records, 4294967296 blocks
level 7: 4294967296 records, 536870912 blocks
level 8: 536870912 records, 67108864 blocks
level 9: 67108864 records, 8388608 blocks
level 10: 8388608 records, 1048576 blocks
level 11: 1048576 records, 131072 blocks
level 12: 131072 records, 16384 blocks
level 13: 16384 records, 2048 blocks
level 14: 2048 records, 256 blocks
level 15: 256 records, 32 blocks
level 16: 32 records, 4 blocks
level 17: 4 records, 1 block
18 levels, 1286742750677285 blocks total

log2(1286742750677285) is 50, so your choice of 53 is fine. Though this
will far exceed the max merkle tree height anyway.

> + * tree size is ((128^8 − 1)/(128 − 1)) = 567*10^12 blocks. This should fit in 53
> + * bits address space.
> + *
> + * At this Merkle tree size we can cover 295EB large file. This is much larger
> + * than the currently supported file size.
> + *
> + * As we allocate some pagecache space for fsverity tree we need to limit
> + * maximum fsverity file size.
> + */
> +#define XFS_FSVERITY_MTREE_OFFSET (1ULL << 53)
> +#define XFS_FSVERITY_MTREE_MASK (XFS_FSVERITY_MTREE_OFFSET - 1)

This is part of the ondisk format, so it belongs in xfs_fs.h.

--D

> +#ifdef CONFIG_FS_VERITY
> +extern const struct fsverity_operations xfs_fsverity_ops;
> +#endif /* CONFIG_FS_VERITY */
> +
> +#endif /* __XFS_FSVERITY_H__ */
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index d7e2b902ef5c..033e1a71e64b 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -404,6 +404,12 @@ static inline bool xfs_inode_can_hw_atomic_write(const struct xfs_inode *ip)
> */
> #define XFS_IREMAPPING (1U << 15)
>
> +/*
> + * fs-verity's Merkle tree is under construction. The file is read-only, the
> + * only writes happening is the ones with Merkle tree blocks.
> + */
> +#define XFS_VERITY_CONSTRUCTION (1U << 16)
> +
> /* All inode state flags related to inode reclaim. */
> #define XFS_ALL_IRECLAIM_FLAGS (XFS_IRECLAIMABLE | \
> XFS_IRECLAIM | \
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 7b1ace75955c..10fb23ac672a 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -30,6 +30,7 @@
> #include "xfs_filestream.h"
> #include "xfs_quota.h"
> #include "xfs_sysfs.h"
> +#include "xfs_fsverity.h"
> #include "xfs_ondisk.h"
> #include "xfs_rmap_item.h"
> #include "xfs_refcount_item.h"
> @@ -54,6 +55,7 @@
> #include <linux/fs_context.h>
> #include <linux/fs_parser.h>
> #include <linux/fsverity.h>
> +#include <linux/iomap.h>
>
> static const struct super_operations xfs_super_operations;
>
> @@ -1711,6 +1713,9 @@ xfs_fs_fill_super(
> sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> #endif
> sb->s_op = &xfs_super_operations;
> +#ifdef CONFIG_FS_VERITY
> + sb->s_vop = &xfs_fsverity_ops;
> +#endif
>
> /*
> * Delay mount work if the debug hook is set. This is debug
> @@ -1964,10 +1969,25 @@ xfs_fs_fill_super(
> xfs_set_resuming_quotaon(mp);
> mp->m_qflags &= ~XFS_QFLAGS_MNTOPTS;
>
> + if (xfs_has_verity(mp))
> + xfs_warn(mp,
> + "EXPERIMENTAL fsverity feature in use. Use at your own risk!");
> +
> error = xfs_mountfs(mp);
> if (error)
> goto out_filestream_unmount;
>
> +#ifdef CONFIG_FS_VERITY
> + /*
> + * Don't use a high priority workqueue like the other fsverity
> + * implementations because that will lead to conflicts with the xfs log
> + * workqueue.
> + */
> + error = iomap_init_fsverity(mp->m_super, 0, 0);
> + if (error)
> + goto out_unmount;
> +#endif
> +
> root = igrab(VFS_I(mp->m_rootip));
> if (!root) {
> error = -ENOENT;
>
> --
> 2.50.0
>
>