On Tue, Apr 02, 2024 at 09:34:53AM -0700, Darrick J. Wong wrote:
> On Tue, Apr 02, 2024 at 10:42:44AM +0200, Andrey Albershteyn wrote:
> > On 2024-03-29 17:39:27, Darrick J. Wong wrote:
> > > From: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> > >
> > > Add integration with fs-verity. The XFS store fs-verity metadata in
> > > the extended file attributes. The metadata consist of verity
> > > descriptor and Merkle tree blocks.
> > >
> > > The descriptor is stored under "vdesc" extended attribute. The
> > > Merkle tree blocks are stored under binary indexes which are offsets
> > > into the Merkle tree.
> > >
> > > 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).
> > >
> > > The verification on read is done in read path of iomap.
> > >
> > > Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> > > Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > [djwong: replace caching implementation with an xarray, other cleanups]
> > > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > > ---
> > > fs/xfs/Makefile | 2
> > > fs/xfs/libxfs/xfs_attr.c | 41 +++
> > > fs/xfs/libxfs/xfs_attr.h | 1
> > > fs/xfs/libxfs/xfs_da_format.h | 14 +
> > > fs/xfs/libxfs/xfs_ondisk.h | 3
> > > fs/xfs/libxfs/xfs_verity.c | 58 ++++
> > > fs/xfs/libxfs/xfs_verity.h | 13 +
> > > fs/xfs/xfs_fsverity.c | 559 +++++++++++++++++++++++++++++++++++++++++
> > > fs/xfs/xfs_fsverity.h | 20 +
> > > fs/xfs/xfs_icache.c | 4
> > > fs/xfs/xfs_inode.h | 5
> > > fs/xfs/xfs_super.c | 17 +
> > > fs/xfs/xfs_trace.h | 32 ++
> > > 13 files changed, 769 insertions(+)
> > > create mode 100644 fs/xfs/libxfs/xfs_verity.c
> > > create mode 100644 fs/xfs/libxfs/xfs_verity.h
> > > create mode 100644 fs/xfs/xfs_fsverity.c
> > > create mode 100644 fs/xfs/xfs_fsverity.h
> > >
> > >
>
> <snip>
>
> > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > > index 5a202706fc4a4..70c5700132b3e 100644
> > > --- a/fs/xfs/xfs_inode.h
> > > +++ b/fs/xfs/xfs_inode.h
> > > @@ -96,6 +96,9 @@ typedef struct xfs_inode {
> > > spinlock_t i_ioend_lock;
> > > struct work_struct i_ioend_work;
> > > struct list_head i_ioend_list;
> > > +#ifdef CONFIG_FS_VERITY
> > > + struct xarray i_merkle_blocks;
> > > +#endif
> >
> > So, is this fine like this or do you plan to change it to per-ag
> > mapping? I suppose Christoph against adding it to inodes [1]
> >
> > [1]: https://lore.kernel.org/linux-xfs/ZfecSzBoVDW5328l@xxxxxxxxxxxxx/
>
> Still working on it. hch and I have been nitpicking the parent pointers
> patchset. I think a per-ag rhashtable would work in principle, but I
> don't know how well it will handle a 128-bit key.
Update: works fine, and now we don't need to add 16 bytes of overhead to
every xfs_inode everywhere.
--D
> --D
>
> > > } xfs_inode_t;
> > >
> > > static inline bool xfs_inode_on_unlinked_list(const struct xfs_inode *ip)
> > > @@ -391,6 +394,8 @@ static inline bool xfs_inode_needs_cow_around(struct xfs_inode *ip)
> > > */
> > > #define XFS_IREMAPPING (1U << 15)
> > >
> > > +#define XFS_VERITY_CONSTRUCTION (1U << 16) /* merkle tree construction */
> > > +
> > > /* 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 42a1e1f23d3b3..4e398884c46ae 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"
> > > @@ -53,6 +54,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;
> > >
> > > @@ -672,6 +674,8 @@ 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);
> > > + if (fsverity_active(inode))
> > > + xfs_fsverity_cache_drop(ip);
> > > fsverity_cleanup_inode(inode);
> > > xfs_inode_mark_reclaimable(ip);
> > > }
> > > @@ -1534,6 +1538,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
> > > @@ -1775,10 +1782,20 @@ xfs_fs_fill_super(
> > > xfs_warn(mp,
> > > "EXPERIMENTAL parent pointer feature enabled. Use at your own risk!");
> > >
> > > + if (xfs_has_verity(mp))
> > > + xfs_alert(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
> > > + error = iomap_init_fsverity(mp->m_super);
> > > + if (error)
> > > + goto out_unmount;
> > > +#endif
> > > +
> > > root = igrab(VFS_I(mp->m_rootip));
> > > if (!root) {
> > > error = -ENOENT;
> > > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> > > index e2992b0115ad2..86a8702c1e27c 100644
> > > --- a/fs/xfs/xfs_trace.h
> > > +++ b/fs/xfs/xfs_trace.h
> > > @@ -5908,6 +5908,38 @@ TRACE_EVENT(xfs_growfs_check_rtgeom,
> > > );
> > > #endif /* CONFIG_XFS_RT */
> > >
> > > +#ifdef CONFIG_FS_VERITY
> > > +DECLARE_EVENT_CLASS(xfs_fsverity_cache_class,
> > > + TP_PROTO(struct xfs_inode *ip, unsigned long key, unsigned long caller_ip),
> > > + TP_ARGS(ip, key, caller_ip),
> > > + TP_STRUCT__entry(
> > > + __field(dev_t, dev)
> > > + __field(xfs_ino_t, ino)
> > > + __field(unsigned long, key)
> > > + __field(void *, caller_ip)
> > > + ),
> > > + TP_fast_assign(
> > > + __entry->dev = ip->i_mount->m_super->s_dev;
> > > + __entry->ino = ip->i_ino;
> > > + __entry->key = key;
> > > + __entry->caller_ip = (void *)caller_ip;
> > > + ),
> > > + TP_printk("dev %d:%d ino 0x%llx key 0x%lx caller %pS",
> > > + MAJOR(__entry->dev), MINOR(__entry->dev),
> > > + __entry->ino,
> > > + __entry->key,
> > > + __entry->caller_ip)
> > > +)
> > > +
> > > +#define DEFINE_XFS_FSVERITY_CACHE_EVENT(name) \
> > > +DEFINE_EVENT(xfs_fsverity_cache_class, name, \
> > > + TP_PROTO(struct xfs_inode *ip, unsigned long key, unsigned long caller_ip), \
> > > + TP_ARGS(ip, key, caller_ip))
> > > +DEFINE_XFS_FSVERITY_CACHE_EVENT(xfs_fsverity_cache_load);
> > > +DEFINE_XFS_FSVERITY_CACHE_EVENT(xfs_fsverity_cache_store);
> > > +DEFINE_XFS_FSVERITY_CACHE_EVENT(xfs_fsverity_cache_drop);
> > > +#endif /* CONFIG_XFS_VERITY */
> > > +
> > > #endif /* _TRACE_XFS_H */
> > >
> > > #undef TRACE_INCLUDE_PATH
> > >
> >
> > --
> > - Andrey
> >
> >
>
