On Fri, Oct 06, 2023 at 08:49:10PM +0200, Andrey Albershteyn wrote:
> The read IO path provides callout for configuring ioend. This allows
> filesystem to add verification of completed BIOs. One of such tasks
> is verification against fs-verity tree when pages were read. iomap
> allows using custom bio_set with submit_bio() to add ioend
> processing. The xfs_prepare_read_ioend() configures bio->bi_end_io
> which places verification task in the workqueue. The task does
> fs-verity verification and then call back to the iomap to finish IO.
>
> This patch adds callouts implementation to verify pages with
> fs-verity. Also implements folio operation .verify_folio for direct
> folio verification by fs-verity.
>
> Signed-off-by: Andrey Albershteyn <aalbersh@xxxxxxxxxx>
> ---
> fs/xfs/xfs_aops.c | 84 ++++++++++++++++++++++++++++++++++++++++++++--
> fs/xfs/xfs_aops.h | 2 ++
> fs/xfs/xfs_linux.h | 1 +
> fs/xfs/xfs_super.c | 9 ++++-
> 4 files changed, 93 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index b413a2dbcc18..fceb0c3de61f 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -26,6 +26,8 @@ struct xfs_writepage_ctx {
> unsigned int cow_seq;
> };
>
> +static struct bio_set xfs_read_ioend_bioset;
> +
> static inline struct xfs_writepage_ctx *
> XFS_WPC(struct iomap_writepage_ctx *ctx)
> {
> @@ -548,19 +550,97 @@ xfs_vm_bmap(
> return iomap_bmap(mapping, block, &xfs_read_iomap_ops);
> }
>
> +static void
> +xfs_read_work_end_io(
> + struct work_struct *work)
> +{
> + struct iomap_read_ioend *ioend =
> + container_of(work, struct iomap_read_ioend, work);
> + struct bio *bio = &ioend->read_inline_bio;
> +
> + fsverity_verify_bio(bio);
> + iomap_read_end_io(bio);
> + /*
> + * The iomap_read_ioend has been freed by bio_put() in
> + * iomap_read_end_io()
> + */
> +}
> +
> +static void
> +xfs_read_end_io(
> + struct bio *bio)
> +{
> + struct iomap_read_ioend *ioend =
> + container_of(bio, struct iomap_read_ioend, read_inline_bio);
> + struct xfs_inode *ip = XFS_I(ioend->io_inode);
> +
> + WARN_ON_ONCE(!queue_work(ip->i_mount->m_postread_workqueue,
> + &ioend->work));
If queue_work fails we should EIO the read.
> +}
> +
> +static int
> +xfs_verify_folio(
> + struct folio *folio,
> + loff_t pos,
> + unsigned int len)
> +{
> + if (fsverity_verify_blocks(folio, len, pos))
> + return 0;
> + return -EFSCORRUPTED;
> +}
> +
> +int
> +xfs_init_iomap_bioset(void)
Probably should be marked __init, right?
> +{
> + return bioset_init(&xfs_read_ioend_bioset,
> + 4 * (PAGE_SIZE / SECTOR_SIZE),
> + offsetof(struct iomap_read_ioend, read_inline_bio),
> + BIOSET_NEED_BVECS);
Also, there's nothing specific to XFS in this bioset, is there?
Shouldn't this be in fs/iomap/buffered-io.c and not XFS?
> +}
> +
> +void
> +xfs_free_iomap_bioset(void)
> +{
> + bioset_exit(&xfs_read_ioend_bioset);
> +}
> +
> +static void
> +xfs_submit_read_bio(
> + const struct iomap_iter *iter,
> + struct bio *bio,
> + loff_t file_offset)
> +{
> + struct iomap_read_ioend *ioend;
> +
> + ioend = container_of(bio, struct iomap_read_ioend, read_inline_bio);
> + ioend->io_inode = iter->inode;
> + if (fsverity_active(ioend->io_inode)) {
> + INIT_WORK(&ioend->work, &xfs_read_work_end_io);
> + ioend->read_inline_bio.bi_end_io = &xfs_read_end_io;
> + }
> +
> + submit_bio(bio);
> +}
> +
> +static const struct iomap_readpage_ops xfs_readpage_ops = {
> + .verify_folio = &xfs_verify_folio,
> + .submit_io = &xfs_submit_read_bio,
> + .bio_set = &xfs_read_ioend_bioset,
> +};
> +
> STATIC int
> xfs_vm_read_folio(
> struct file *unused,
> struct folio *folio)
> {
> - return iomap_read_folio(folio, &xfs_read_iomap_ops, NULL);
> + return iomap_read_folio(folio, &xfs_read_iomap_ops, &xfs_readpage_ops);
Leave the ops parameter as NULL for non-verity filesystems to avoid the
overhead of indirect calls. Work data partitions aren't going to enable
verity.
--D
> }
>
> STATIC void
> xfs_vm_readahead(
> struct readahead_control *rac)
> {
> - iomap_readahead(rac, &xfs_read_iomap_ops, NULL);
> + iomap_readahead(rac, &xfs_read_iomap_ops, &xfs_readpage_ops);
> }
>
> static int
> diff --git a/fs/xfs/xfs_aops.h b/fs/xfs/xfs_aops.h
> index e0bd68419764..fa7c512b2717 100644
> --- a/fs/xfs/xfs_aops.h
> +++ b/fs/xfs/xfs_aops.h
> @@ -10,5 +10,7 @@ extern const struct address_space_operations xfs_address_space_operations;
> extern const struct address_space_operations xfs_dax_aops;
>
> int xfs_setfilesize(struct xfs_inode *ip, xfs_off_t offset, size_t size);
> +int xfs_init_iomap_bioset(void);
> +void xfs_free_iomap_bioset(void);
>
> #endif /* __XFS_AOPS_H__ */
> diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
> index e9d317a3dafe..ee213c6dfcaf 100644
> --- a/fs/xfs/xfs_linux.h
> +++ b/fs/xfs/xfs_linux.h
> @@ -64,6 +64,7 @@ typedef __u32 xfs_nlink_t;
> #include <linux/xattr.h>
> #include <linux/mnt_idmapping.h>
> #include <linux/debugfs.h>
> +#include <linux/fsverity.h>
>
> #include <asm/page.h>
> #include <asm/div64.h>
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 5e1ec5978176..3cdb642961f4 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -2375,11 +2375,17 @@ init_xfs_fs(void)
> if (error)
> goto out_remove_dbg_kobj;
>
> - error = register_filesystem(&xfs_fs_type);
> + error = xfs_init_iomap_bioset();
> if (error)
> goto out_qm_exit;
> +
> + error = register_filesystem(&xfs_fs_type);
> + if (error)
> + goto out_iomap_bioset;
> return 0;
>
> + out_iomap_bioset:
> + xfs_free_iomap_bioset();
> out_qm_exit:
> xfs_qm_exit();
> out_remove_dbg_kobj:
> @@ -2412,6 +2418,7 @@ init_xfs_fs(void)
> STATIC void __exit
> exit_xfs_fs(void)
> {
> + xfs_free_iomap_bioset();
> xfs_qm_exit();
> unregister_filesystem(&xfs_fs_type);
> #ifdef DEBUG
> --
> 2.40.1
>
