[PATCH v2 0/4] iomap: fix error handling regressions

[PATCH v2 0/4] iomap: fix error handling regressions

[Andrea Parri]

Adding Carlos Maiolino to the To: list since patch 2 (new in v2) touches
fs/xfs/ outside the iomap directory.

Fix three independent error handling regressions in iomap:

 - clear the writeback context after a failed ->writeback_submit() call,
   preventing an already completed ioend from being submitted again;
 - preserve an iomap iteration error when iomap_fiemap() has a pending
   extent to emit; and
 - preserve a direct I/O data path error when sub-block tail zeroing
   succeeds.

The three fixes have no ordering dependencies and can be applied or
backported independently.  Each issue was reproduced before and after
its respective fix.

Patch 2 is not a fix and carries no stable tag: it adds an XFS error
tag (requested by Christoph) that lets patch 1's ->writeback_submit()
failure be injected without the boot-param hack used for the original
reproduction.  I used it to write an xfstest reproducing the bug (list
corruption caught via CONFIG_DEBUG_LIST on an unfixed kernel, clean
writeback failure on a fixed one); I'll send that separately to
fstests@vger.kernel.org once this series is out for review.

Changes since v1:
 - Patch 1: reworded the added comment to fit 80 columns (Christoph);
   answered Christoph's request for a reproducer in the commit message
   (fault-injected ->writeback_submit() failure on a reflinked XFS file,
   reliably hits list_add double add on the unfixed kernel); confirmed
   with Christoph that the final ->writeback_submit() call in
   iomap_writepages() needs no equivalent fix, since wpc is never reused
   after it; picked up the version-scoped stable tag Darrick suggested;
   picked up Brian's Reviewed-by.
 - Patch 2: new, added per Christoph's request.
 - Patch 3 (was 2/3): no code change; picked up the version-scoped
   stable tag Darrick suggested; picked up Reviewed-by tags from Brian,
   Darrick, and Christoph.
 - Patch 4 (was 3/3): reworked per Christoph's suggestion to drop the
   zerror/ret handling entirely instead of just fixing its type, since
   iomap_dio_zero() can only fail via a can't-happen WARN_ON_ONCE()
   assert; this also resolves Darrick's question about the local
   variable's type, since there's no longer a local variable to type.

Andrea Parri (4):
  iomap: don't resubmit an ioend after ->writeback_submit() failed
  xfs: add an error tag to inject a ->writeback_submit() failure
  iomap: don't lose a fiemap iteration error when emitting the last
    extent
  iomap: don't lose a failed direct I/O bio's error when zeroing the
    tail

 fs/iomap/direct-io.c         | 19 +++++++------------
 fs/iomap/fiemap.c            |  8 ++++----
 fs/iomap/ioend.c             | 10 +++++++++-
 fs/xfs/libxfs/xfs_errortag.h |  6 ++++--
 fs/xfs/xfs_reflink.c         |  5 +++++
 5 files changed, 29 insertions(+), 19 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.53.0

[PATCH v2 1/4] iomap: don't resubmit an ioend after ->writeback_submit() failed

[Andrea Parri]

iomap_add_to_ioend() submits the pending ioend through
->writeback_submit() before allocating a new one for the current range.
When the submission fails the helper completes the ioend with an error,
but iomap_add_to_ioend() returns the error without clearing
wpc->wb_ctx.  iomap_writepages() then submits whatever wpc->wb_ctx
points to, so the already completed ioend is submitted a second time.

For XFS the second bio_endio() lands in xfs_end_bio(), which
list_add_tail()s the already linked ioend into ip->i_ioend_list.  This
corrupts the list and leaves a use-after-free/double-free window against
the ioend completion worker.  Reproduced with a fault-injected
->writeback_submit() failure on a reflinked XFS file with several
CoW writeback ranges in flight: the unfixed kernel hits repeated
"list_add double add" warnings from __list_add_valid_or_report(),
the fixed kernel fails writeback cleanly.

Clear wpc->wb_ctx when ->writeback_submit() fails.  The old
iomap_submit_ioend() cleared the context unconditionally; that clear was
lost when submission moved to iomap_ioend_writeback_submit().  The final
->writeback_submit() call in iomap_writepages() needs no equivalent fix:
it is the last thing the function does before returning, and every
caller allocates its iomap_writepage_ctx on the stack for a single call,
so wpc->wb_ctx is never read again afterwards.

Fixes: f4fa7981fa26 ("iomap: hide ioends from the generic writeback code")
Cc: <stable@vger.kernel.org> # v6.17
Reviewed-by: Brian Foster <bfoster@redhat.com>
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/iomap/ioend.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index 7bbbb417f9152..32ae292a84cbe 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -246,8 +246,16 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
 new_ioend:
 		if (ioend) {
 			error = wpc->ops->writeback_submit(wpc, 0);
-			if (error)
+			if (error) {
+				/*
+				 * ->writeback_submit() completed the ioend
+				 * with an error, so drop the stale context;
+				 * iomap_writepages() would otherwise submit
+				 * it a second time.
+				 */
+				wpc->wb_ctx = NULL;
 				return error;
+			}
 		}
 		wpc->wb_ctx = ioend = iomap_alloc_ioend(wpc, pos, ioend_flags);
 	}
-- 
2.53.0

[PATCH v2 2/4] xfs: add an error tag to inject a ->writeback_submit() failure

[Andrea Parri]

The only in-tree way for xfs_writeback_submit() to fail is a failing
xfs_reflink_convert_cow(), which requires a shared (reflinked) CoW
extent.  There was previously no way to exercise that failure path
from userspace, which made it hard to write an xfstest for it, e.g.
for "iomap: don't resubmit an ioend after ->writeback_submit()
failed".

Add XFS_ERRTAG_WB_COW_CONVERT_ERROR to force xfs_reflink_convert_cow()
to fail with -EIO, so that ->writeback_submit() failure and its
callers' error handling can be exercised with error injection alone,
without needing a corrupted or racy COW fork.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/xfs/libxfs/xfs_errortag.h | 6 ++++--
 fs/xfs/xfs_reflink.c         | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index 6de207fed2d89..e6b0864051b0a 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -75,7 +75,8 @@
 #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
 #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
 #define XFS_ERRTAG_ZONE_RESET				47
-#define XFS_ERRTAG_MAX					48
+#define XFS_ERRTAG_WB_COW_CONVERT_ERROR		48
+#define XFS_ERRTAG_MAX					49
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -137,7 +138,8 @@ XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
 XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
 XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
 XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
-XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
+XFS_ERRTAG(ZONE_RESET,		zone_reset,		1) \
+XFS_ERRTAG(WB_COW_CONVERT_ERROR, wb_cow_convert_error,	1)
 #endif /* XFS_ERRTAG */
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 4801361366359..104542b10d9e0 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -34,6 +34,8 @@
 #include "xfs_rtalloc.h"
 #include "xfs_rtgroup.h"
 #include "xfs_metafile.h"
+#include "xfs_errortag.h"
+#include "xfs_error.h"
 
 /*
  * Copy on Write of Shared Blocks
@@ -346,6 +348,9 @@ xfs_reflink_convert_cow(
 
 	ASSERT(count != 0);
 
+	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_WB_COW_CONVERT_ERROR))
+		return -EIO;
+
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	error = xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
 	xfs_iunlock(ip, XFS_ILOCK_EXCL);
-- 
2.53.0

[PATCH v2 3/4] iomap: don't lose a fiemap iteration error when emitting the last extent

[Andrea Parri]

iomap_fiemap() emits extents one behind: iomap_fiemap_iter() flushes the
previous extent and remembers the current one, and the remembered extent
is written with FIEMAP_EXTENT_LAST after the iteration loop.

That final flush overwrites ret, so when ->iomap_begin() fails partway
through the iteration the error is replaced by the result of
iomap_to_fiemap() (zero on success) and iomap_fiemap() returns success
with a truncated extent list whose last entry is wrongly marked as the
last extent in the file.  The pre-iomap_iter code returned the error
from inside the loop, before flushing the pending extent.

Check for the iteration error before flushing the pending extent, so
that real errors are propagated and only a successful iteration emits
the final FIEMAP_EXTENT_LAST extent.  -ENOENT (no mapping) is still not
an error, and the pending extent is still emitted in that case.

Fixes: 7892386d3571 ("iomap: switch iomap_fiemap to use iomap_iter")
Cc: <stable@vger.kernel.org> # v5.15
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/iomap/fiemap.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index d11dadff82865..54b824b7edb5c 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -76,15 +76,15 @@ int iomap_fiemap(struct inode *inode, struct fiemap_extent_info *fi,
 	while ((ret = iomap_iter(&iter, ops)) > 0)
 		iter.status = iomap_fiemap_iter(&iter, fi, &prev);
 
+	/* inode with no (attribute) mapping will give ENOENT */
+	if (ret < 0 && ret != -ENOENT)
+		return ret;
+
 	if (prev.type != IOMAP_HOLE) {
 		ret = iomap_to_fiemap(fi, &prev, FIEMAP_EXTENT_LAST);
 		if (ret < 0)
 			return ret;
 	}
-
-	/* inode with no (attribute) mapping will give ENOENT */
-	if (ret < 0 && ret != -ENOENT)
-		return ret;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(iomap_fiemap);
-- 
2.53.0

[PATCH v2 4/4] iomap: don't lose a failed direct I/O bio's error when zeroing the tail

[Andrea Parri]

iomap_dio_bio_iter() falls through to the sub-block tail zeroing when
the data bio submission fails, so that the rest of the block is still
zeroed and stale data is not exposed.  The zeroing result was assigned
to ret, which overwrote the submission error with the successful
zeroing result (zero) and the failed write was reported as success.

iomap_dio_zero() can only return an error from a can't-happen
WARN_ON_ONCE() (nr_vecs exceeding BIO_MAX_VECS, which the existing
comment there says "shall never be reached" for any in-tree
filesystem), so it isn't a real runtime failure worth reporting to
userspace, let alone one worth losing the actual submission error for.
Make iomap_dio_zero() return void and drop the error handling at both
call sites instead of threading the result through a separate
variable.

Fixes: 10553a91652d ("iomap: fix iomap_dio_zero() for fs bs > system page size")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
 fs/iomap/direct-io.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 8b4039d16ce89..e00995c296c79 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -296,8 +296,9 @@ u32 iomap_finish_ioend_direct(struct iomap_ioend *ioend)
 	return vec_count;
 }
 
-static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
-		loff_t pos, unsigned len)
+static void iomap_dio_zero(const struct iomap_iter *iter,
+			   struct iomap_dio *dio, loff_t pos,
+			   unsigned int len)
 {
 	struct inode *inode = file_inode(dio->iocb->ki_filp);
 	struct bio *bio;
@@ -305,14 +306,14 @@ static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 	int nr_vecs = max(1, i_blocksize(inode) / folio_size(zero_folio));
 
 	if (!len)
-		return 0;
+		return;
 
 	/*
 	 * This limit shall never be reached as most filesystems have a
 	 * maximum blocksize of 64k.
 	 */
 	if (WARN_ON_ONCE(nr_vecs > BIO_MAX_VECS))
-		return -EINVAL;
+		return;
 
 	bio = iomap_dio_alloc_bio(iter, dio, nr_vecs,
 				  REQ_OP_WRITE | REQ_SYNC | REQ_IDLE);
@@ -328,8 +329,6 @@ static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
 		len -= io_len;
 	}
 	iomap_dio_submit_bio(iter, dio, bio, pos);
-
-	return 0;
 }
 
 static ssize_t iomap_dio_bio_iter_one(struct iomap_iter *iter,
@@ -541,10 +540,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	if (need_zeroout) {
 		/* zero out from the start of the block to the write offset */
 		pad = pos & (fs_block_size - 1);
-
-		ret = iomap_dio_zero(iter, dio, pos - pad, pad);
-		if (ret)
-			goto out;
+		iomap_dio_zero(iter, dio, pos - pad, pad);
 	}
 
 	do {
@@ -582,8 +578,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 		/* zero out from the end of the write to the end of the block */
 		pad = pos & (fs_block_size - 1);
 		if (pad)
-			ret = iomap_dio_zero(iter, dio, pos,
-					     fs_block_size - pad);
+			iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
 	}
 out:
 	/* Undo iter limitation to current extent */
-- 
2.53.0

Re: [PATCH v2 1/4] iomap: don't resubmit an ioend after ->writeback_submit() failed

[Darrick J. Wong]

On Thu, Sep 24, 2026 at 11:11:51AM +0200, Andrea Parri wrote:
> iomap_add_to_ioend() submits the pending ioend through
> ->writeback_submit() before allocating a new one for the current range.
> When the submission fails the helper completes the ioend with an error,
> but iomap_add_to_ioend() returns the error without clearing
> wpc->wb_ctx.  iomap_writepages() then submits whatever wpc->wb_ctx
> points to, so the already completed ioend is submitted a second time.
> 
> For XFS the second bio_endio() lands in xfs_end_bio(), which
> list_add_tail()s the already linked ioend into ip->i_ioend_list.  This
> corrupts the list and leaves a use-after-free/double-free window against
> the ioend completion worker.  Reproduced with a fault-injected
> ->writeback_submit() failure on a reflinked XFS file with several
> CoW writeback ranges in flight: the unfixed kernel hits repeated
> "list_add double add" warnings from __list_add_valid_or_report(),
> the fixed kernel fails writeback cleanly.
> 
> Clear wpc->wb_ctx when ->writeback_submit() fails.  The old
> iomap_submit_ioend() cleared the context unconditionally; that clear was
> lost when submission moved to iomap_ioend_writeback_submit().  The final
> ->writeback_submit() call in iomap_writepages() needs no equivalent fix:
> it is the last thing the function does before returning, and every
> caller allocates its iomap_writepage_ctx on the stack for a single call,
> so wpc->wb_ctx is never read again afterwards.
> 
> Fixes: f4fa7981fa26 ("iomap: hide ioends from the generic writeback code")
> Cc: <stable@vger.kernel.org> # v6.17
> Reviewed-by: Brian Foster <bfoster@redhat.com>
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  fs/iomap/ioend.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
> index 7bbbb417f9152..32ae292a84cbe 100644
> --- a/fs/iomap/ioend.c
> +++ b/fs/iomap/ioend.c
> @@ -246,8 +246,16 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
>  new_ioend:
>  		if (ioend) {
>  			error = wpc->ops->writeback_submit(wpc, 0);
> -			if (error)
> +			if (error) {
> +				/*
> +				 * ->writeback_submit() completed the ioend
> +				 * with an error, so drop the stale context;
> +				 * iomap_writepages() would otherwise submit
> +				 * it a second time.
> +				 */
> +				wpc->wb_ctx = NULL;

I looked around the codebase and saw that iomap_writepages also calls
->writeback_submit.  Does that need to null out wb_ctx?

My guess is that none of the callers do anything with wpc after
iomap_writepages returns so it's not harming anyone, but we should drop
the stale context too, right?

--D

>  				return error;
> +			}
>  		}
>  		wpc->wb_ctx = ioend = iomap_alloc_ioend(wpc, pos, ioend_flags);
>  	}
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 2/4] xfs: add an error tag to inject a ->writeback_submit() failure

[Darrick J. Wong]

On Thu, Sep 24, 2026 at 11:11:52AM +0200, Andrea Parri wrote:
> The only in-tree way for xfs_writeback_submit() to fail is a failing
> xfs_reflink_convert_cow(), which requires a shared (reflinked) CoW
> extent.  There was previously no way to exercise that failure path
> from userspace, which made it hard to write an xfstest for it, e.g.
> for "iomap: don't resubmit an ioend after ->writeback_submit()
> failed".
> 
> Add XFS_ERRTAG_WB_COW_CONVERT_ERROR to force xfs_reflink_convert_cow()
> to fail with -EIO, so that ->writeback_submit() failure and its
> callers' error handling can be exercised with error injection alone,
> without needing a corrupted or racy COW fork.
> 
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>

Looks good to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_errortag.h | 6 ++++--
>  fs/xfs/xfs_reflink.c         | 5 +++++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 6de207fed2d89..e6b0864051b0a 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -75,7 +75,8 @@
>  #define XFS_ERRTAG_METAFILE_RESV_CRITICAL		45
>  #define XFS_ERRTAG_FORCE_ZERO_RANGE			46
>  #define XFS_ERRTAG_ZONE_RESET				47
> -#define XFS_ERRTAG_MAX					48
> +#define XFS_ERRTAG_WB_COW_CONVERT_ERROR		48
> +#define XFS_ERRTAG_MAX					49
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -137,7 +138,8 @@ XFS_ERRTAG(WRITE_DELAY_MS,	write_delay_ms,		3000) \
>  XFS_ERRTAG(EXCHMAPS_FINISH_ONE,	exchmaps_finish_one,	1) \
>  XFS_ERRTAG(METAFILE_RESV_CRITICAL, metafile_resv_crit,	4) \
>  XFS_ERRTAG(FORCE_ZERO_RANGE,	force_zero_range,	4) \
> -XFS_ERRTAG(ZONE_RESET,		zone_reset,		1)
> +XFS_ERRTAG(ZONE_RESET,		zone_reset,		1) \
> +XFS_ERRTAG(WB_COW_CONVERT_ERROR, wb_cow_convert_error,	1)
>  #endif /* XFS_ERRTAG */
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 4801361366359..104542b10d9e0 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -34,6 +34,8 @@
>  #include "xfs_rtalloc.h"
>  #include "xfs_rtgroup.h"
>  #include "xfs_metafile.h"
> +#include "xfs_errortag.h"
> +#include "xfs_error.h"
>  
>  /*
>   * Copy on Write of Shared Blocks
> @@ -346,6 +348,9 @@ xfs_reflink_convert_cow(
>  
>  	ASSERT(count != 0);
>  
> +	if (XFS_TEST_ERROR(mp, XFS_ERRTAG_WB_COW_CONVERT_ERROR))
> +		return -EIO;
> +
>  	xfs_ilock(ip, XFS_ILOCK_EXCL);
>  	error = xfs_reflink_convert_cow_locked(ip, offset_fsb, count_fsb);
>  	xfs_iunlock(ip, XFS_ILOCK_EXCL);
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 4/4] iomap: don't lose a failed direct I/O bio's error when zeroing the tail

[Darrick J. Wong]

On Thu, Sep 24, 2026 at 11:11:54AM +0200, Andrea Parri wrote:
> iomap_dio_bio_iter() falls through to the sub-block tail zeroing when
> the data bio submission fails, so that the rest of the block is still
> zeroed and stale data is not exposed.  The zeroing result was assigned
> to ret, which overwrote the submission error with the successful
> zeroing result (zero) and the failed write was reported as success.
> 
> iomap_dio_zero() can only return an error from a can't-happen
> WARN_ON_ONCE() (nr_vecs exceeding BIO_MAX_VECS, which the existing
> comment there says "shall never be reached" for any in-tree
> filesystem), so it isn't a real runtime failure worth reporting to
> userspace, let alone one worth losing the actual submission error for.
> Make iomap_dio_zero() return void and drop the error handling at both
> call sites instead of threading the result through a separate
> variable.
> 
> Fixes: 10553a91652d ("iomap: fix iomap_dio_zero() for fs bs > system page size")
> Cc: stable@vger.kernel.org

Cc: <stable@vger.kernel.org> # v6.12

> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
> ---
>  fs/iomap/direct-io.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 8b4039d16ce89..e00995c296c79 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -296,8 +296,9 @@ u32 iomap_finish_ioend_direct(struct iomap_ioend *ioend)
>  	return vec_count;
>  }
>  
> -static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
> -		loff_t pos, unsigned len)
> +static void iomap_dio_zero(const struct iomap_iter *iter,
> +			   struct iomap_dio *dio, loff_t pos,
> +			   unsigned int len)
>  {
>  	struct inode *inode = file_inode(dio->iocb->ki_filp);
>  	struct bio *bio;
> @@ -305,14 +306,14 @@ static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
>  	int nr_vecs = max(1, i_blocksize(inode) / folio_size(zero_folio));
>  
>  	if (!len)
> -		return 0;
> +		return;
>  
>  	/*
>  	 * This limit shall never be reached as most filesystems have a
>  	 * maximum blocksize of 64k.
>  	 */
>  	if (WARN_ON_ONCE(nr_vecs > BIO_MAX_VECS))
> -		return -EINVAL;
> +		return;
>  
>  	bio = iomap_dio_alloc_bio(iter, dio, nr_vecs,
>  				  REQ_OP_WRITE | REQ_SYNC | REQ_IDLE);
> @@ -328,8 +329,6 @@ static int iomap_dio_zero(const struct iomap_iter *iter, struct iomap_dio *dio,
>  		len -= io_len;
>  	}
>  	iomap_dio_submit_bio(iter, dio, bio, pos);
> -
> -	return 0;
>  }
>  
>  static ssize_t iomap_dio_bio_iter_one(struct iomap_iter *iter,
> @@ -541,10 +540,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	if (need_zeroout) {
>  		/* zero out from the start of the block to the write offset */
>  		pad = pos & (fs_block_size - 1);
> -
> -		ret = iomap_dio_zero(iter, dio, pos - pad, pad);
> -		if (ret)
> -			goto out;
> +		iomap_dio_zero(iter, dio, pos - pad, pad);
>  	}
>  
>  	do {
> @@ -582,8 +578,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  		/* zero out from the end of the write to the end of the block */
>  		pad = pos & (fs_block_size - 1);
>  		if (pad)
> -			ret = iomap_dio_zero(iter, dio, pos,
> -					     fs_block_size - pad);
> +			iomap_dio_zero(iter, dio, pos, fs_block_size - pad);

I think this is the original cause of the bug, right?  We might have
already had a nonzero ret, and the assignment here blows that away.
Right?

If the answer to that is yes, then
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

(I should probably whine about how changing the function signature of
iomap_dio_zero should be a separate patch to make it more obvious what's
the actual fix...)

--D

>  	}
>  out:
>  	/* Undo iter limitation to current extent */
> -- 
> 2.53.0
> 
> 

Re: [PATCH v2 1/4] iomap: don't resubmit an ioend after ->writeback_submit() failed

[Andrea Parri]

On Thu, Sep 24, 2026 at 11:49:10AM -0700, Darrick J. Wong wrote:
> I looked around the codebase and saw that iomap_writepages also calls
> ->writeback_submit.  Does that need to null out wb_ctx?
> 
> My guess is that none of the callers do anything with wpc after
> iomap_writepages returns so it's not harming anyone, but we should drop
> the stale context too, right?

Your guess is right: that ->writeback_submit() call is the last thing
iomap_writepages() does, and all of its callers (xfs, gfs2, zonefs,
exfat, fuse, ntfs, ntfs3) have the wpc on the stack for that single
call, so wb_ctx is never looked at again.  Christoph made the same
point on v1 [1], and the last paragraph of the commit message now
spells it out.

Clearing it there as well would be harmless but dead code, so I'd
rather leave it out; happy to add it if you feel strongly about it.

[1] https://lore.kernel.org/all/arITXTsUhrA0N-qr@infradead.org/

Thanks!
  Andrea

Re: [PATCH v2 4/4] iomap: don't lose a failed direct I/O bio's error when zeroing the tail

[Andrea Parri]

On Thu, Sep 24, 2026 at 11:53:17AM -0700, Darrick J. Wong wrote:
> > Cc: stable@vger.kernel.org
> 
> Cc: <stable@vger.kernel.org> # v6.12

Fixed for v3.

> > @@ -582,8 +578,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
> >  		/* zero out from the end of the write to the end of the block */
> >  		pad = pos & (fs_block_size - 1);
> >  		if (pad)
> > -			ret = iomap_dio_zero(iter, dio, pos,
> > -					     fs_block_size - pad);
> > +			iomap_dio_zero(iter, dio, pos, fs_block_size - pad);
> 
> I think this is the original cause of the bug, right?  We might have
> already had a nonzero ret, and the assignment here blows that away.
> Right?

Right.  When iomap_dio_bio_iter_one() fails we break out of the loop
with ret < 0 and fall through to the tail zeroing, and this assignment
replaced that error with iomap_dio_zero()'s 0.

> If the answer to that is yes, then
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thanks, picked up for v3.

> (I should probably whine about how changing the function signature of
> iomap_dio_zero should be a separate patch to make it more obvious what's
> the actual fix...)

Fair point.  FWIW, before 10553a91652d iomap_dio_zero() returned void
and neither call site touched ret; that commit made it return int and
added both "ret = iomap_dio_zero(...)" assignments.  So the signature
change here is really undoing the part of 10553a91652d that introduced
the bug, and keeping it in one patch makes the stable backport
self-contained.  If you'd still prefer a one-line fix followed by the
void conversion, I'm happy to split it.

Thanks!
  Andrea