Re: [PATCH RESEND 1/2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()y

[Gao Xiang <xiang@kernel.org>]

Hi Sadman,

On Tue, Oct 06, 2026 at 02:04:47PM -0400, Sadman Fahmid via B4 Relay wrote:
> From: Sadman Fahmid <sadmanf@meta.com>
> 
> erofs_io_xcopy() falls back to a read/pwrite loop for whatever
> copy_file_range() and sendfile() did not copy.  If the source reaches
> EOF before `len` bytes have been copied, erofs_io_read() returns 0,
> `len` never shrinks and mkfs spins forever instead of reporting the
> error.
> 
> This is easy to hit with a damaged source, e.g. rebuilding with
> `mkfs.erofs --clean=data out.img src.img` where src.img is truncated
> in the middle of a file's data, or when a source file shrinks while
> mkfs runs.  Return -EIO instead.
> 
> While at it, keep calling copy_file_range() until it has copied the
> whole range or fails.  A single call may copy less than requested
> (e.g. the kernel caps each call at MAX_RW_COUNT, just under 2 GiB,
> while callers pass chunks of up to 4 GiB), and the remainder was
> previously copied by sendfile() instead of being reflinked.
> 
> Signed-off-by: Sadman Fahmid <sadmanf@meta.com>
> ---
>  lib/io.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/io.c b/lib/io.c
> index 5c5449f..038fa51 100644
> --- a/lib/io.c
> +++ b/lib/io.c
> @@ -642,9 +642,13 @@ int erofs_io_xcopy(struct erofs_vfile *vout, off_t pos,
>  		off_t ret __maybe_unused;
>  
>  #ifdef HAVE_COPY_FILE_RANGE
> -		ret = copy_file_range(vin->fd, NULL, vout->fd, &pos, len, 0);
> -		if (ret > 0)
> +		while (len) {
> +			ret = copy_file_range(vin->fd, NULL, vout->fd, &pos,
> +					      len, 0);
> +			if (ret <= 0)
> +				break;
>  			len -= ret;
> +		}
>  #endif
>  #if defined(HAVE_SYS_SENDFILE_H) && defined(HAVE_SENDFILE)
>  		if (len && !noseek) {
> @@ -660,21 +664,20 @@ int erofs_io_xcopy(struct erofs_vfile *vout, off_t pos,
>  #endif
>  	}
>  
> -	do {
> +	while (len) {
>  		char buf[32768];
> -		int ret = min_t(unsigned int, len, sizeof(buf));


Thanks for the patch.

I just have one question, could the overflow here be resolved
by
		int ret = min_t(s64, len, sizeof(buf));

Since ret won't exceed sizeof(buf) now.

Otherwise it looks good to me.

Thanks,
Gao Xiang