Re: [PATCH v2 5/22] iomap: integrate fs-verity verification into iomap's read path

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Jan 12, 2026 at 03:50:26PM +0100, Andrey Albershteyn wrote:
> This patch adds fs-verity verification into iomap's read path. After
> BIO's io operation is complete the data are verified against
> fs-verity's Merkle tree. Verification work is done in a separate
> workqueue.
> 
> The read path ioend iomap_read_ioend are stored side by side with
> BIOs if FS_VERITY is enabled.
> 
> Signed-off-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
>  fs/iomap/bio.c         | 66 ++++++++++++++++++++++++++++++++++++++++++++++++----
>  fs/iomap/buffered-io.c | 12 ++++++++-
>  fs/iomap/ioend.c       | 41 +++++++++++++++++++++++++++++++-
>  include/linux/iomap.h  | 11 ++++++++
>  4 files changed, 123 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
> index fc045f2e4c..ac6c16b1f8 100644
> --- a/fs/iomap/bio.c
> +++ b/fs/iomap/bio.c
> @@ -5,6 +5,7 @@
>   */
>  #include <linux/iomap.h>
>  #include <linux/pagemap.h>
> +#include <linux/fsverity.h>
>  #include "internal.h"
>  #include "trace.h"
>  
> @@ -18,6 +19,60 @@
>  	bio_put(bio);
>  }
>  
> +#ifdef CONFIG_FS_VERITY

Should all this stuff go into fs/iomap/fsverity.c instead of ifdef'd
around the iomap code?

<shrug>

> +static void
> +iomap_read_fsverify_end_io_work(struct work_struct *work)
> +{
> +	struct iomap_fsverity_bio *fbio =
> +		container_of(work, struct iomap_fsverity_bio, work);
> +
> +	fsverity_verify_bio(&fbio->bio);
> +	iomap_read_end_io(&fbio->bio);
> +}
> +
> +static void
> +iomap_read_fsverity_end_io(struct bio *bio)
> +{
> +	struct iomap_fsverity_bio *fbio =
> +		container_of(bio, struct iomap_fsverity_bio, bio);
> +
> +	INIT_WORK(&fbio->work, iomap_read_fsverify_end_io_work);
> +	fsverity_enqueue_verify_work(&fbio->work);
> +}
> +
> +static struct bio *
> +iomap_fsverity_read_bio_alloc(struct inode *inode, struct block_device *bdev,
> +			    int nr_vecs, gfp_t gfp)
> +{
> +	struct bio *bio;
> +
> +	bio = bio_alloc_bioset(bdev, nr_vecs, REQ_OP_READ, gfp,
> +			iomap_fsverity_bioset);
> +	if (bio)
> +		bio->bi_end_io = iomap_read_fsverity_end_io;
> +	return bio;
> +}
> +
> +#else
> +# define iomap_fsverity_read_bio_alloc(...)	(NULL)
> +# define iomap_fsverity_tree_end_align(...)	(false)
> +#endif /* CONFIG_FS_VERITY */
> +
> +static struct bio *iomap_read_bio_alloc(struct inode *inode,
> +		const struct iomap *iomap, int nr_vecs, gfp_t gfp)
> +{
> +	struct bio *bio;
> +	struct block_device *bdev = iomap->bdev;
> +
> +	if (!(iomap->flags & IOMAP_F_BEYOND_EOF) && fsverity_active(inode))
> +		return iomap_fsverity_read_bio_alloc(inode, bdev, nr_vecs, gfp);
> +
> +	bio = bio_alloc(bdev, nr_vecs, REQ_OP_READ, gfp);
> +	if (bio)
> +		bio->bi_end_io = iomap_read_end_io;
> +	return bio;
> +}
> +
>  static void iomap_bio_submit_read(struct iomap_read_folio_ctx *ctx)
>  {
>  	struct bio *bio = ctx->read_ctx;
> @@ -42,26 +97,27 @@
>  	    !bio_add_folio(bio, folio, plen, poff)) {
>  		gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
>  		gfp_t orig_gfp = gfp;
> -		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
>  
>  		if (bio)
>  			submit_bio(bio);
>  
>  		if (ctx->rac) /* same as readahead_gfp_mask */
>  			gfp |= __GFP_NORETRY | __GFP_NOWARN;
> -		bio = bio_alloc(iomap->bdev, bio_max_segs(nr_vecs), REQ_OP_READ,
> -				     gfp);
> +		bio = iomap_read_bio_alloc(iter->inode, iomap,
> +				bio_max_segs(DIV_ROUND_UP(length, PAGE_SIZE)),
> +				gfp);
> +
>  		/*
>  		 * If the bio_alloc fails, try it again for a single page to
>  		 * avoid having to deal with partial page reads.  This emulates
>  		 * what do_mpage_read_folio does.
>  		 */
>  		if (!bio)
> -			bio = bio_alloc(iomap->bdev, 1, REQ_OP_READ, orig_gfp);
> +			bio = iomap_read_bio_alloc(iter->inode, iomap, 1,
> +						   orig_gfp);
>  		if (ctx->rac)
>  			bio->bi_opf |= REQ_RAHEAD;
>  		bio->bi_iter.bi_sector = sector;
> -		bio->bi_end_io = iomap_read_end_io;
>  		bio_add_folio_nofail(bio, folio, plen, poff);
>  		ctx->read_ctx = bio;
>  	}
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 79d1c97f02..481f7e1cff 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -8,6 +8,7 @@
>  #include <linux/writeback.h>
>  #include <linux/swap.h>
>  #include <linux/migrate.h>
> +#include <linux/fsverity.h>
>  #include "internal.h"
>  #include "trace.h"
>  
> @@ -532,10 +533,19 @@
>  		if (plen == 0)
>  			return 0;
>  
> +		/* end of fs-verity region*/
> +		if ((iomap->flags & IOMAP_F_BEYOND_EOF) && (iomap->type == IOMAP_HOLE)) {

Overly long line.

Also, when do we get the combination of BEYOND_EOF && HOLE?  Is that for
sparse regions in only the merkle tree?  IIRC (and I could be wrong)
fsverity still wants to checksum sparse holes in the regular file data,
right?

> +			folio_zero_range(folio, poff, plen);
> +			iomap_set_range_uptodate(folio, poff, plen);
> +		}
>  		/* zero post-eof blocks as the page may be mapped */
> -		if (iomap_block_needs_zeroing(iter, pos) &&
> +		else if (iomap_block_needs_zeroing(iter, pos) &&

		} else if (...

(nitpicking indentation)

>  		    !(iomap->flags & IOMAP_F_BEYOND_EOF)) {
>  			folio_zero_range(folio, poff, plen);
> +			if (fsverity_active(iter->inode) &&
> +			    !fsverity_verify_blocks(folio, plen, poff)) {
> +				return -EIO;
> +			}
>  			iomap_set_range_uptodate(folio, poff, plen);
>  		} else {
>  			if (!*bytes_submitted)
> diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
> index 86f44922ed..30c0de3c75 100644
> --- a/fs/iomap/ioend.c
> +++ b/fs/iomap/ioend.c
> @@ -9,6 +9,8 @@
>  #include "internal.h"
>  #include "trace.h"
>  
> +#define IOMAP_POOL_SIZE		(4 * (PAGE_SIZE / SECTOR_SIZE))

How do we arrive at this pool size?  How is it important to have a
larger bio reserve pool for *larger* base page sizes?

--D

> +
>  struct bio_set iomap_ioend_bioset;
>  EXPORT_SYMBOL_GPL(iomap_ioend_bioset);
>  
> @@ -423,9 +425,46 @@
>  }
>  EXPORT_SYMBOL_GPL(iomap_split_ioend);
>  
> +#ifdef CONFIG_FS_VERITY
> +struct bio_set *iomap_fsverity_bioset;
> +EXPORT_SYMBOL_GPL(iomap_fsverity_bioset);
> +int iomap_fsverity_init_bioset(void)
> +{
> +	struct bio_set *bs, *old;
> +	int error;
> +
> +	bs = kzalloc(sizeof(*bs), GFP_KERNEL);
> +	if (!bs)
> +		return -ENOMEM;
> +
> +	error = bioset_init(bs, IOMAP_POOL_SIZE,
> +			    offsetof(struct iomap_fsverity_bio, bio),
> +			    BIOSET_NEED_BVECS);
> +	if (error) {
> +		kfree(bs);
> +		return error;
> +	}
> +
> +	/*
> +	 * This has to be atomic as readaheads can race to create the
> +	 * bioset.  If someone set the pointer before us, we drop ours.
> +	 */
> +	old = cmpxchg(&iomap_fsverity_bioset, NULL, bs);
> +	if (old) {
> +		bioset_exit(bs);
> +		kfree(bs);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(iomap_fsverity_init_bioset);
> +#else
> +# define iomap_fsverity_init_bioset(...)	(-EOPNOTSUPP)
> +#endif
> +
>  static int __init iomap_ioend_init(void)
>  {
> -	return bioset_init(&iomap_ioend_bioset, 4 * (PAGE_SIZE / SECTOR_SIZE),
> +	return bioset_init(&iomap_ioend_bioset, IOMAP_POOL_SIZE,
>  			   offsetof(struct iomap_ioend, io_bio),
>  			   BIOSET_NEED_BVECS);
>  }
> diff --git a/include/linux/iomap.h b/include/linux/iomap.h
> index 7a7e31c499..b451ab3426 100644
> --- a/include/linux/iomap.h
> +++ b/include/linux/iomap.h
> @@ -342,6 +342,17 @@
>  		iter->srcmap.type == IOMAP_MAPPED;
>  }
>  
> +#ifdef CONFIG_FS_VERITY
> +extern struct bio_set *iomap_fsverity_bioset;
> +
> +struct iomap_fsverity_bio {
> +	struct work_struct	work;
> +	struct bio		bio;
> +};
> +
> +int iomap_fsverity_init_bioset(void);
> +#endif
> +
>  ssize_t iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *from,
>  		const struct iomap_ops *ops,
>  		const struct iomap_write_ops *write_ops, void *private);
> 
> -- 
> - Andrey
> 
>