Linux-EROFS Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Gao Xiang <xiang@kernel.org>
To: sadmanf@meta.com
Cc: linux-erofs@lists.ozlabs.org, Gao Xiang <xiang@kernel.org>,
	Chao Yu <chao@kernel.org>
Subject: Re: [PATCH v2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()
Date: Wed, 7 Oct 2026 22:15:40 +0200	[thread overview]
Message-ID: <asaobA7u4wcb1jxV@XiangdeMacBook-Pro.local> (raw)
In-Reply-To: <20261007-round1-v2-1-3ffc17bb9028@meta.com>

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



      reply	other threads:[~2026-10-07 20:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-06 18:04 [PATCH RESEND 0/2] erofs-utils: lib: fix a short-read hang and an fd leak Sadman Fahmid via B4 Relay
2026-10-06 18:04 ` [PATCH RESEND 1/2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy() Sadman Fahmid via B4 Relay
2026-10-06 20:51   ` [PATCH RESEND 1/2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy()y Gao Xiang
2026-10-06 18:04 ` [PATCH RESEND 2/2] erofs-utils: lib: close source fd on early errors in erofs_mkfs_begin_nondirectory() Sadman Fahmid via B4 Relay
2026-10-06 20:57   ` Gao Xiang
2026-10-07 19:15 ` [PATCH v2] erofs-utils: lib: fail instead of hanging on short reads in erofs_io_xcopy() Sadman Fahmid via B4 Relay
2026-10-07 20:15   ` Gao Xiang [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=asaobA7u4wcb1jxV@XiangdeMacBook-Pro.local \
    --to=xiang@kernel.org \
    --cc=chao@kernel.org \
    --cc=linux-erofs@lists.ozlabs.org \
    --cc=sadmanf@meta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox