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

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

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