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