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

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

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