[PATCH 0/2] iomap: fixes for short copies and integrity bio sizes

[PATCH 0/2] iomap: fixes for short copies and integrity bio sizes

[Christian Brauner]

Two iomap fixes that came out of Josef's AI bug scans.

The first one is old. For a buffered write into a hole, an unwritten
extent or past EOF __iomap_write_begin() marks the zeroed blocks
uptodate before the copy from userspace. If the copy comes up short the
uncopied part of the write stays behind in an uptodate block:

        ftruncate(fd, 4096);
        pwrite(fd, p, 3000, 100);    /* 16 bytes at p, then PROT_NONE */
        pread(fd, buf, 4096, 0);

That returns 2984 bytes of whatever was in the page before. Willy
reported this in 2024 and Zhang Yi posted a fix but it never got merged.
I went with the same fix. Josef's version zeroed the partially written
blocks completely and only marked those uptodate. That works too but
it's a lot more code for the same result.

Willy suggested back then to drop the uptodate marking from
__iomap_write_begin() completely. I didn't do that. Blocks that were
read from disk are valid and keeping them uptodate means a retried short
copy doesn't have to read them again.

The second one is from v7.1. When the filesystem handles the integrity
metadata the bio size check wraps if a single chunk is larger than
max_integrity_io_size(). That limit is capped by the maximum segment
size, so it's 64 KiB on lpfc and qla2xxx, and XFS writeback of a large
preallocated file happily builds a single 300 MiB bio and overruns the
2 MiB integrity buffer.

Note that a single chunk above the limit still gets a bio of its own.
That's fine with 4k pages. But with 64k pages a folio can be 128 MiB
which needs 4 MiB of PI with 16 bytes of metadata per 512 byte interval.
That's not new though and needs a separate fix.

Both are tested in a VM. Without the first patch the short copy
reproducer leaks on XFS with 4k and 1k blocks and on ext4 with buffered
iomap, and an fsx-like checker with faulting user buffers fails within
a few hundred operations. With it both pass. The second patch was tested
with scsi_debug DIF/DIX patched to a 64 KiB maximum segment size.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (1):
      iomap: don't mark unwritten parts of a folio uptodate

Josef Bacik (1):
      iomap: keep large chunks from defeating the bio cap

 fs/iomap/bio.c         | 2 +-
 fs/iomap/buffered-io.c | 3 ++-
 fs/iomap/internal.h    | 9 +++++++++
 fs/iomap/ioend.c       | 3 +--
 4 files changed, 13 insertions(+), 4 deletions(-)
---
base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
change-id: 20261029-work-iomap-ai-fixes-5c962e384d74

[PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Christian Brauner]

For a buffered write into a hole, an unwritten or new extent, or past
EOF __iomap_write_begin() zeroes the parts of the blocks outside of the
write and then marks the blocks uptodate. That includes the range of the
write itself which still holds whatever was in the folio before. It only
becomes valid once the copy from userspace has filled it in.

But the copy can come up short and __iomap_write_end() only rejects a
short copy if the folio isn't uptodate. If the zeroed blocks were all
that was missing the folio is uptodate by then, the short copy is
accepted, and the rest of the range is exposed to readers and can end up
on disk. A source buffer that ends in an unmapped page is enough:

        ftruncate(fd, 4096);
        pwrite(fd, p, 3000, 100);    /* 16 bytes at p, then PROT_NONE */
        pread(fd, buf, 4096, 0);

The write returns 16 and the read returns 2984 bytes of whatever was in
that page before. If other blocks of the folio aren't uptodate the short
copy is rejected but the zeroed blocks keep their uptodate bits and the
retry with the copied length leaves the same bytes behind.

Only mark blocks uptodate in __iomap_write_begin() after reading them.
__iomap_write_end() marks the zeroed blocks uptodate once the copy has
succeeded and rejects a short copy.

Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
fix (cf. [2]) but it never got merged.

[1]: https://lore.kernel.org/all/ZqsN5ouQTEc1KAzV@casper.infradead.org
[2]: https://lore.kernel.org/all/20240812121159.3775074-6-yi.zhang@huaweicloud.com

Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
Reported-by: Josef Bacik <josef@toxicpanda.com>
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/iomap/buffered-io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 907873073b03..d6e2f1ded5ca 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -920,6 +920,7 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
 		if (iomap_block_needs_zeroing(iter, block_start)) {
 			if (WARN_ON_ONCE(iter->flags & IOMAP_UNSHARE))
 				return -EIO;
+			/* uptodate once copied, see __iomap_write_end() */
 			folio_zero_segments(folio, poff, from, to, poff + plen);
 		} else {
 			const struct iomap *iomap = iomap_iter_srcmap(iter);
@@ -943,8 +944,8 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
 
 			if (iomap->flags & IOMAP_F_ZERO_TAIL)
 				folio_zero_segment(folio, to, poff + plen);
+			iomap_set_range_uptodate(folio, poff, plen);
 		}
-		iomap_set_range_uptodate(folio, poff, plen);
 	} while ((block_start += plen) < block_end);
 
 	return 0;

-- 
2.53.0

[PATCH 2/2] iomap: keep large chunks from defeating the bio cap

[Christian Brauner]

From: Josef Bacik <josef@toxicpanda.com>

For integrity I/O a bio must not grow beyond max_integrity_io_size()
because the integrity buffer is a single allocation of at most 2 MiB.
Writeback and buffered reads check that before adding to a bio:

        bio->bi_iter.bi_size > iomap_max_bio_size(iomap) - len

The subtraction is done in size_t and wraps if len alone is above the
limit. The check then never fires and the bio grows until it runs out
of bio_vecs.

That's easy to hit. The limit is also capped by the maximum segment size
which is 64 KiB for SCSI hosts that don't set it, such as lpfc and
qla2xxx, and XFS uses folios of up to 2 MiB. Writeback of a preallocated
300 MiB file then builds a single bio that needs 4.7 MiB of PI with 512
byte sectors and 8 byte tuples. That's more than kmalloc() can do, so
bio_integrity_alloc_buf() falls back to a 2 MiB mempool element and
generating the PI writes 2.7 MiB past its end.

Check the length on its own first. A chunk above the limit still gets a
bio of its own which is never larger than a folio.

Fixes: 0b10a370529c ("iomap: support T10 protection information")
Cc: stable@vger.kernel.org # v7.1+
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/iomap/bio.c      | 2 +-
 fs/iomap/internal.h | 9 +++++++++
 fs/iomap/ioend.c    | 3 +--
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
index 153b779949e9..256564ebe238 100644
--- a/fs/iomap/bio.c
+++ b/fs/iomap/bio.c
@@ -109,7 +109,7 @@ int iomap_bio_read_folio_range(const struct iomap_iter *iter,
 
 	if (!bio ||
 	    bio_end_sector(bio) != iomap_sector(&iter->iomap, iter->pos) ||
-	    bio->bi_iter.bi_size > iomap_max_bio_size(&iter->iomap) - plen ||
+	    !iomap_bio_has_room(&iter->iomap, bio, plen) ||
 	    !bio_add_folio(bio, folio, plen, offset_in_folio(folio, iter->pos)))
 		iomap_read_alloc_bio(iter, ctx, plen);
 	return 0;
diff --git a/fs/iomap/internal.h b/fs/iomap/internal.h
index 74e898b196dc..e69b3ac02cc2 100644
--- a/fs/iomap/internal.h
+++ b/fs/iomap/internal.h
@@ -17,6 +17,15 @@ static inline size_t iomap_max_bio_size(const struct iomap *iomap)
 	return BIO_MAX_SIZE;
 }
 
+/* Can @len more bytes go into @bio? */
+static inline bool iomap_bio_has_room(const struct iomap *iomap,
+		const struct bio *bio, size_t len)
+{
+	size_t max_size = iomap_max_bio_size(iomap);
+
+	return len <= max_size && bio->bi_iter.bi_size <= max_size - len;
+}
+
 u32 iomap_finish_ioend_buffered_read(struct iomap_ioend *ioend);
 u32 iomap_finish_ioend_direct(struct iomap_ioend *ioend);
 
diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index 92b89fbf95b9..4da248ff99f8 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -133,8 +133,7 @@ static bool iomap_can_add_to_ioend(struct iomap_writepage_ctx *wpc, loff_t pos,
 {
 	struct iomap_ioend *ioend = wpc->wb_ctx;
 
-	if (ioend->io_bio.bi_iter.bi_size >
-	    iomap_max_bio_size(&wpc->iomap) - map_len)
+	if (!iomap_bio_has_room(&wpc->iomap, &ioend->io_bio, map_len))
 		return false;
 	if (ioend_flags & IOMAP_IOEND_BOUNDARY)
 		return false;

-- 
2.53.0

Re: [PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Christoph Hellwig]

On Thu, Oct 29, 2026 at 11:34:48AM +0100, Christian Brauner wrote:
> on disk. A source buffer that ends in an unmapped page is enough:
> 
>         ftruncate(fd, 4096);
>         pwrite(fd, p, 3000, 100);    /* 16 bytes at p, then PROT_NONE */
>         pread(fd, buf, 4096, 0);

Can we wire this up to xfstests, please?

> Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
> fix (cf. [2]) but it never got merged.

Should we credit the original submission then?

Re: [PATCH 2/2] iomap: keep large chunks from defeating the bio cap

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Christian Brauner]

On Thu, Oct 29, 2026 at 01:12:20PM +0100, Christoph Hellwig wrote:
> On Thu, Oct 29, 2026 at 11:34:48AM +0100, Christian Brauner wrote:
> > on disk. A source buffer that ends in an unmapped page is enough:
> > 
> >         ftruncate(fd, 4096);
> >         pwrite(fd, p, 3000, 100);    /* 16 bytes at p, then PROT_NONE */
> >         pread(fd, buf, 4096, 0);
> 
> Can we wire this up to xfstests, please?

I have an endless queue of additional bugs to go through so I can say
yes but I can't commit to any timeframe nor the ability to remember.

> > Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
> > fix (cf. [2]) but it never got merged.
> 
> Should we credit the original submission then?

It's references in the commit message already. Tell me what you think we
need to see there and I'll amend in-tree.

Re: [PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Christoph Hellwig]

On Thu, Oct 29, 2026 at 01:20:09PM +0100, Christian Brauner wrote:
> > Can we wire this up to xfstests, please?
> 
> I have an endless queue of additional bugs to go through so I can say
> yes but I can't commit to any timeframe nor the ability to remember.
> 
> > > Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
> > > fix (cf. [2]) but it never got merged.
> > 
> > Should we credit the original submission then?
> 
> It's references in the commit message already. Tell me what you think we
> need to see there and I'll amend in-tree.

Usually that just means taking the older patch instead of the newer one.
Although chances are that this one might not apply anymore, in which
case taking the new one should be ok.

Re: [PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Joanne Koong]

On Thu, Oct 29, 2026 at 3:35 AM Christian Brauner <brauner@kernel.org> wrote:
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 907873073b03..d6e2f1ded5ca 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -920,6 +920,7 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
>                 if (iomap_block_needs_zeroing(iter, block_start)) {
>                         if (WARN_ON_ONCE(iter->flags & IOMAP_UNSHARE))
>                                 return -EIO;
> +                       /* uptodate once copied, see __iomap_write_end() */
>                         folio_zero_segments(folio, poff, from, to, poff + plen);
>                 } else {
>                         const struct iomap *iomap = iomap_iter_srcmap(iter);
> @@ -943,8 +944,8 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
>
>                         if (iomap->flags & IOMAP_F_ZERO_TAIL)
>                                 folio_zero_segment(folio, to, poff + plen);
> +                       iomap_set_range_uptodate(folio, poff, plen);
>                 }
> -               iomap_set_range_uptodate(folio, poff, plen);
>         } while ((block_start += plen) < block_end);
>
>         return 0;
>

The fix looks good to me.

Reviewed-by: Joanne Koong <joannelkoong@gmail.com>

Re: [PATCH 1/2] iomap: don't mark unwritten parts of a folio uptodate

[Zhang Yi]

On 10/29/2026 6:34 PM, Christian Brauner wrote:
> For a buffered write into a hole, an unwritten or new extent, or past
> EOF __iomap_write_begin() zeroes the parts of the blocks outside of the
> write and then marks the blocks uptodate. That includes the range of the
> write itself which still holds whatever was in the folio before. It only
> becomes valid once the copy from userspace has filled it in.
> 
> But the copy can come up short and __iomap_write_end() only rejects a
> short copy if the folio isn't uptodate. If the zeroed blocks were all
> that was missing the folio is uptodate by then, the short copy is
> accepted, and the rest of the range is exposed to readers and can end up
> on disk. A source buffer that ends in an unmapped page is enough:
> 
>         ftruncate(fd, 4096);
>         pwrite(fd, p, 3000, 100);    /* 16 bytes at p, then PROT_NONE */
>         pread(fd, buf, 4096, 0);
> 
> The write returns 16 and the read returns 2984 bytes of whatever was in
> that page before. If other blocks of the folio aren't uptodate the short
> copy is rejected but the zeroed blocks keep their uptodate bits and the
> retry with the copied length leaves the same bytes behind.
> 
> Only mark blocks uptodate in __iomap_write_begin() after reading them.
> __iomap_write_end() marks the zeroed blocks uptodate once the copy has
> succeeded and rejects a short copy.
> 
> Willy reported this back in 2024 (cf. [1]) and Zhang Yi posted the same
> fix (cf. [2]) but it never got merged.
> 

Ha, thanks for picking this fix back up. I had missed this patch while
sending the iomap fixes.

> [1]: https://lore.kernel.org/all/ZqsN5ouQTEc1KAzV@casper.infradead.org
> [2]: https://lore.kernel.org/all/20240812121159.3775074-6-yi.zhang@huaweicloud.com
> 
> Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads")
> Reported-by: Josef Bacik <josef@toxicpanda.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/iomap/buffered-io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 907873073b03..d6e2f1ded5ca 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -920,6 +920,7 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
>  		if (iomap_block_needs_zeroing(iter, block_start)) {
>  			if (WARN_ON_ONCE(iter->flags & IOMAP_UNSHARE))
>  				return -EIO;
> +			/* uptodate once copied, see __iomap_write_end() */
>  			folio_zero_segments(folio, poff, from, to, poff + plen);
>  		} else {
>  			const struct iomap *iomap = iomap_iter_srcmap(iter);
> @@ -943,8 +944,8 @@ static int __iomap_write_begin(const struct iomap_iter *iter,
>  
>  			if (iomap->flags & IOMAP_F_ZERO_TAIL)
>  				folio_zero_segment(folio, to, poff + plen);
> +			iomap_set_range_uptodate(folio, poff, plen);
>  		}
> -		iomap_set_range_uptodate(folio, poff, plen);
>  	} while ((block_start += plen) < block_end);
>  
>  	return 0;
>