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

[PATCH 1/3] 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.

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().

Fixes: f4fa7981fa26c ("iomap: hide ioends from the generic writeback code")
Cc: stable@vger.kernel.org
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..bb8575768d888 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

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

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:31:31AM +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.
> 
> 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().
> 
> Fixes: f4fa7981fa26c ("iomap: hide ioends from the generic writeback code")
> Cc: stable@vger.kernel.org

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

> 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..bb8575768d888 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;

Do you need to do the same thing for the ->writeback_submit call in
iomap_writepages?

--D

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

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

[Christoph Hellwig]

On Mon, Sep 21, 2026 at 03:29:53PM -0700, Darrick J. Wong wrote:
> > +				 */
> > +				wpc->wb_ctx = NULL;
> 
> Do you need to do the same thing for the ->writeback_submit call in
> iomap_writepages?

No, after that the wpc is not reused.

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

[Andrea Parri]

> > Do you need to do the same thing for the ->writeback_submit call in
> > iomap_writepages?
> 
> No, after that the wpc is not reused.

Matches what I found too, thanks for confirming, Christoph.

Darrick, I'll pick up the v6.17 stable tag for v2.

Thanks!
  Andrea

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

[Christoph Hellwig]

On Mon, Sep 21, 2026 at 10:31:31AM +0200, Andrea Parri wrote:
> 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.

Do you have a reproducer for this using some kind of error injection?

>  		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

Overly long line here.

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

[Andrea Parri]

> Do you have a reproducer for this using some kind of error injection?

Yes.  xfs_reflink_convert_cow() is the only in-tree way
xfs_writeback_submit() can fail, so I added a boot param that forces
it to return -EIO, reflinked a file, dirtied several widely
separated CoW regions so writeback has to submit more than one
ioend, and let background writeback run under the fault injector.

On the unfixed kernel this reliably hits "list_add double add" in
__list_add_valid_or_report(), from the stale ioend's second
bio_endio() landing in xfs_end_bio() and relinking it into
ip->i_ioend_list. With the fix applied, writeback just fails cleanly
and there's no corruption.  Happy to describe the harness in more
detail if useful, it's throwaway boot-param/script scaffolding, not
something I'd want to send as-is.


> Overly long line here.

Fixed, will rewrap that comment for v2.

Thanks!
  Andrea

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

[Christoph Hellwig]

On Tue, Sep 22, 2026 at 11:03:09AM +0200, Andrea Parri wrote:
> Yes.  xfs_reflink_convert_cow() is the only in-tree way
> xfs_writeback_submit() can fail, so I added a boot param that forces
> it to return -EIO, reflinked a file, dirtied several widely
> separated CoW regions so writeback has to submit more than one
> ioend, and let background writeback run under the fault injector.

Can you add an errortag in fs/xfs/libxfs/xfs_errortag.h to inject
such an error and write an xfstest that uses it?

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

[Andrea Parri]

> Can you add an errortag in fs/xfs/libxfs/xfs_errortag.h to inject
> such an error and write an xfstest that uses it?

Looking into it.  Running a few more local iterations both w/ and w/o my
fix, the plan is to include the errortag patch in v2 of this series.

Thanks!
  Andrea

[PATCH 2/3] 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: 7892386d35715 ("iomap: switch iomap_fiemap to use iomap_iter")
Cc: stable@vger.kernel.org
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

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

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:31:32AM +0200, Andrea Parri wrote:
> 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: 7892386d35715 ("iomap: switch iomap_fiemap to use iomap_iter")
> Cc: stable@vger.kernel.org

Oooh, an oldie!

Cc: <stable@vger.kernel.org> # v5.15

> Assisted-by: LLM
> Signed-off-by: Andrea Parri <parri.andrea@gmail.com>

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  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
> 

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

[Christoph Hellwig]

Looks good:

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

[PATCH 3/3] 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 is assigned to
ret, which overwrites the submission error with the successful zeroing
result (zero) and the failed write is reported as success.

Store the zeroing result separately and only use it when the data path
did not already fail.

Fixes: 10553a91652d9 ("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 | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 8b4039d16ce89..8ae3fe64e475c 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -581,9 +581,14 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
 	    ((dio->flags & IOMAP_DIO_WRITE) && pos >= i_size_read(inode))) {
 		/* 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);
+		if (pad) {
+			ssize_t zerror;
+
+			zerror = iomap_dio_zero(iter, dio, pos,
+						fs_block_size - pad);
+			if (!ret)
+				ret = zerror;
+		}
 	}
 out:
 	/* Undo iter limitation to current extent */
-- 
2.53.0

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

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 10:31:33AM +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 is assigned to
> ret, which overwrites the submission error with the successful zeroing
> result (zero) and the failed write is reported as success.
> 
> Store the zeroing result separately and only use it when the data path
> did not already fail.
> 
> Fixes: 10553a91652d9 ("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 | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
> index 8b4039d16ce89..8ae3fe64e475c 100644
> --- a/fs/iomap/direct-io.c
> +++ b/fs/iomap/direct-io.c
> @@ -581,9 +581,14 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio)
>  	    ((dio->flags & IOMAP_DIO_WRITE) && pos >= i_size_read(inode))) {
>  		/* 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);
> +		if (pad) {
> +			ssize_t zerror;

Why ssize_t?  iomap_dio_zero returns int, right?  I may have missed
something in my absence, but AFAICT it should be:

			int ret2 = iomap_dio_zero(...);

			if (ret2 && !ret)
				ret = ret2;

--D

> +
> +			zerror = iomap_dio_zero(iter, dio, pos,
> +						fs_block_size - pad);
> +			if (!ret)
> +				ret = zerror;
> +		}
>  	}
>  out:
>  	/* Undo iter limitation to current extent */
> -- 
> 2.53.0
> 

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

[Christoph Hellwig]

On Mon, Sep 21, 2026 at 03:34:32PM -0700, Darrick J. Wong wrote:
> > -		if (pad)
> > -			ret = iomap_dio_zero(iter, dio, pos,
> > -					     fs_block_size - pad);
> > +		if (pad) {
> > +			ssize_t zerror;
> 
> Why ssize_t?  iomap_dio_zero returns int, right?  I may have missed
> something in my absence, but AFAICT it should be:

More importantly iomap_dio_zero can only return an error for a
logic bomb assert using WARN_ON.  I'd suggest to simply return without
an error there and remove the handling of the iomap_dio_zero return
value entirely, as it clearly casues more harm than it helps.

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

[Andrea Parri]

> > Why ssize_t?  iomap_dio_zero returns int, right?
> 
> More importantly iomap_dio_zero can only return an error for a
> logic bomb assert using WARN_ON.  I'd suggest to simply return without
> an error there and remove the handling of the iomap_dio_zero return
> value entirely, as it clearly casues more harm than it helps.

Agreed, that's cleaner than retyping the local.  I'll drop the
zerror/ret handling entirely for v2 and just call iomap_dio_zero()
without capturing its return.

Darrick, that also answers your question, the type mismatch goes
away since there's no local variable left.

Thanks!
  Andrea

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

[Christoph Hellwig]

On Tue, Sep 22, 2026 at 11:10:13AM +0200, Andrea Parri wrote:
> > > Why ssize_t?  iomap_dio_zero returns int, right?
> > 
> > More importantly iomap_dio_zero can only return an error for a
> > logic bomb assert using WARN_ON.  I'd suggest to simply return without
> > an error there and remove the handling of the iomap_dio_zero return
> > value entirely, as it clearly casues more harm than it helps.
> 
> Agreed, that's cleaner than retyping the local.  I'll drop the
> zerror/ret handling entirely for v2 and just call iomap_dio_zero()
> without capturing its return.

Please drop the return value from iomap_dio_zero entirely.

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

[Andrea Parri]

> Please drop the return value from iomap_dio_zero entirely.

Done locally: iomap_dio_zero() now returns void, and both call sites no
longer capture or check a result.

Thanks!
  Andrea