[PATCH RESEND 0/2] erofs-utils: lib: fix a short-read hang and an fd leak

[PATCH RESEND 0/2] erofs-utils: lib: fix a short-read hang and an fd leak

[Sadman Fahmid via B4 Relay]

Patch 1 makes erofs_io_xcopy() return -EIO instead of spinning forever
when the source hits EOF early (e.g. rebuilding from a truncated
image), and keeps calling copy_file_range() for ranges over 2 GiB.

Patch 2 closes the source fd on two early error paths in
erofs_mkfs_begin_nondirectory().

---
Sadman Fahmid (2):
      erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()
      erofs-utils: lib: close source fd on early errors in erofs_mkfs_begin_nondirectory()

 lib/inode.c | 14 +++++++++++---
 lib/io.c    | 29 ++++++++++++++++-------------
 2 files changed, 27 insertions(+), 16 deletions(-)
---
base-commit: 1c230a485f25837cb51d6fc79332f30758d9006c
change-id: 20261006-round1-ae8882b2efc7

Best regards,
-- 
Sadman Fahmid <sadmanf@meta.com>

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

[Sadman Fahmid via B4 Relay]

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));
+		ssize_t nread, ret;
 
-		ret = erofs_io_read(vin, buf, ret);
+		nread = erofs_io_read(vin, buf, min_t(unsigned int, len,
+						      sizeof(buf)));
+		if (nread <= 0)
+			return nread < 0 ? nread : -EIO;
+		ret = erofs_io_pwrite(vout, buf, pos, nread);
 		if (ret < 0)
 			return ret;
-		if (ret > 0) {
-			ret = erofs_io_pwrite(vout, buf, pos, ret);
-			if (ret < 0)
-				return ret;
-			pos += ret;
-		}
-		len -= ret;
-	} while (len);
+		pos += nread;
+		len -= nread;
+	}
 	return 0;
 }
 

-- 
2.53.0-Meta

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

[Gao Xiang]

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

[PATCH RESEND 2/2] erofs-utils: lib: close source fd on early errors in erofs_mkfs_begin_nondirectory()

[Sadman Fahmid via B4 Relay]

From: Sadman Fahmid <sadmanf@meta.com>

If erofs_set_inode_fingerprint() or erofs_prepare_compressed_file()
fails, the source file descriptor opened just above is never closed
since the job carrying it is never queued.  Close it on those paths.

The diskbuf descriptor is left alone as it is owned by the diskbuf.

Signed-off-by: Sadman Fahmid <sadmanf@meta.com>

---
The erofs_begin_compressed_file() failure path is left as is: with
multi-threaded compression, z_erofs_mt_compress() may fail after some
segments have already been queued to workers that read from the fd,
so closing it there would be unsafe.
---
 lib/inode.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/inode.c b/lib/inode.c
index ec96556..489c44e 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -2086,14 +2086,16 @@ static int erofs_mkfs_begin_nondirectory(const struct erofs_mkfs_btctx *btctx,
 
 		ret = erofs_set_inode_fingerprint(inode, ctx.fd, ctx.fpos);
 		if (ret < 0)
-			return ret;
+			goto err_close;
 
 		if (inode->datasource != EROFS_INODE_DATA_SOURCE_REBUILD_BLOB &&
 		    inode->sbi->available_compr_algs &&
 		    erofs_file_is_compressible(im, inode)) {
 			ctx.ictx = erofs_prepare_compressed_file(im, inode);
-			if (IS_ERR(ctx.ictx))
-				return PTR_ERR(ctx.ictx);
+			if (IS_ERR(ctx.ictx)) {
+				ret = PTR_ERR(ctx.ictx);
+				goto err_close;
+			}
 			erofs_bind_compressed_file_with_fd(ctx.ictx,
 							   ctx.fd, ctx.fpos);
 			ret = erofs_begin_compressed_file(ctx.ictx);
@@ -2103,6 +2105,12 @@ static int erofs_mkfs_begin_nondirectory(const struct erofs_mkfs_btctx *btctx,
 	}
 out:
 	return erofs_mkfs_go(btctx, EROFS_MKFS_JOB_NDIR, &ctx, sizeof(ctx));
+
+err_close:
+	/* the diskbuf fd is shared and released along with the diskbuf */
+	if (inode->datasource != EROFS_INODE_DATA_SOURCE_DISKBUF)
+		close(ctx.fd);
+	return ret;
 }
 
 static int erofs_mkfs_handle_inode(const struct erofs_mkfs_btctx *ctx,

-- 
2.53.0-Meta

Re: [PATCH RESEND 2/2] erofs-utils: lib: close source fd on early errors in erofs_mkfs_begin_nondirectory()

[Gao Xiang]

On Tue, Oct 06, 2026 at 02:04:48PM -0400, Sadman Fahmid via B4 Relay wrote:
> From: Sadman Fahmid <sadmanf@meta.com>
> 
> If erofs_set_inode_fingerprint() or erofs_prepare_compressed_file()
> fails, the source file descriptor opened just above is never closed
> since the job carrying it is never queued.  Close it on those paths.
> 
> The diskbuf descriptor is left alone as it is owned by the diskbuf.
> 
> Signed-off-by: Sadman Fahmid <sadmanf@meta.com>
> 
> ---
> The erofs_begin_compressed_file() failure path is left as is: with
> multi-threaded compression, z_erofs_mt_compress() may fail after some
> segments have already been queued to workers that read from the fd,
> so closing it there would be unsafe.


I applied this.  Although yeah, the failure path needs to be revisited
later.

Thanks,
Gao Xiang

[PATCH v2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()

[Sadman Fahmid via B4 Relay]

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>
---
Changes in v2:
- Use min_t(s64, len, sizeof(buf)) and int read/write result
  variables, as suggested by Gao Xiang. Each request is bounded
  to 32 KiB.
- Resend only the short-read fix. Gao Xiang already applied
  the fd-leak fix (patch 2 from v1).
- Link to v1:
  https://lore.kernel.org/r/20261006-round1-v1-0-f46b13e11de5@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..ea3bcbf 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));
+		int nread = min_t(s64, len, sizeof(buf));
+		int ret;
 
-		ret = erofs_io_read(vin, buf, ret);
+		nread = erofs_io_read(vin, buf, nread);
+		if (nread <= 0)
+			return nread < 0 ? nread : -EIO;
+		ret = erofs_io_pwrite(vout, buf, pos, nread);
 		if (ret < 0)
 			return ret;
-		if (ret > 0) {
-			ret = erofs_io_pwrite(vout, buf, pos, ret);
-			if (ret < 0)
-				return ret;
-			pos += ret;
-		}
-		len -= ret;
-	} while (len);
+		pos += nread;
+		len -= nread;
+	}
 	return 0;
 }
 

---
base-commit: 1c230a485f25837cb51d6fc79332f30758d9006c
change-id: 20261006-round1-ae8882b2efc7

Best regards,
-- 
Sadman Fahmid <sadmanf@meta.com>

Re: [PATCH v2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()

[Gao Xiang]

On Wed, Oct 07, 2026 at 03:15:26PM -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>
> ---
> Changes in v2:
> - Use min_t(s64, len, sizeof(buf)) and int read/write result
>   variables, as suggested by Gao Xiang. Each request is bounded
>   to 32 KiB.
> - Resend only the short-read fix. Gao Xiang already applied
>   the fd-leak fix (patch 2 from v1).
> - Link to v1:
>   https://lore.kernel.org/r/20261006-round1-v1-0-f46b13e11de5@meta.com

Hi Sadman,

I applied your patch with minor changes (mainly avoid short
erofs_io_pwrite() ...) If there is any problem, kindly help
feedback.

Thanks,
Gao Xiang

From f4a027130ad7842bfa7b7c29a7226c1181281e50 Mon Sep 17 00:00:00 2001
From: Sadman Fahmid <sadmanf@meta.com>
Date: Wed, 7 Oct 2026 15:15:26 -0400
Subject: [PATCH] erofs-utils: lib: fail instead of hanging on short reads in
 erofs_io_xcopy()

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>
Link: https://patch.msgid.link/20261007-round1-v2-1-3ffc17bb9028@meta.com
[ Gao Xiang: let's avoid short erofs_io_pwrite(). ]
Signed-off-by: Gao Xiang <xiang@kernel.org>
---
 lib/io.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/lib/io.c b/lib/io.c
index 5c5449ff..4bb40ab1 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,19 @@ 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));
-
-		ret = erofs_io_read(vin, buf, ret);
-		if (ret < 0)
-			return ret;
-		if (ret > 0) {
-			ret = erofs_io_pwrite(vout, buf, pos, ret);
-			if (ret < 0)
-				return ret;
-			pos += ret;
-		}
-		len -= ret;
-	} while (len);
+		int ret, nread;
+
+		nread = erofs_io_read(vin, buf, min_t(s64, len, sizeof(buf));
+		if (nread <= 0)
+			return nread < 0 ? nread : -EIO;
+		ret = erofs_io_pwrite(vout, buf, pos, nread);
+		if (ret != nread)
+			return ret < 0 ? ret : -EIO;
+		pos += nread;
+		len -= nread;
+	}
 	return 0;
 }