[PATCH] erofs-utils: lib: fix infinite loop on EOF in erofs_io_xcopy

[Ajay Rajera]

Hi, Thanks for the review.
Okay, I understand it now.
I've just sent out a v2 patch incorporating your suggested changes.

Best regards,
Ajay Rajera

On Fri, 20 Mar 2026 at 21:01, Lucas Karpinski <lkarpinski at nvidia.com> wrote:
>
> On 2026-03-19 8:20 p.m., Ajay Rajera wrote:
> > erofs_io_xcopy() has a fallback do-while loop for when the
> > kernel fast-paths (copy_file_range/sendfile) do not handle all
> > the data.  The loop does:
> >
> >     ret = erofs_io_read(vin, buf, ret);
> >     if (ret < 0)
> >         return ret;
> >     if (ret > 0) { ... pos += ret; }
> >     len -= ret;
> >   } while (len);
> >
> > When erofs_io_read() returns 0 (EOF -- source exhausted before
> > all bytes were copied), only the ret < 0 and ret > 0 branches
> > were handled.  Since ret == 0, `len -= ret` is a no-op and
> > `while (len)` stays true, causing the loop to spin forever at
> > 100% CPU with no error and no progress.
> >
> > This can be triggered when building an EROFS image from an input
> > file that is shorter than expected -- e.g. a truncated source
> > file, a pipe/FIFO that closes early, or a file being modified
> > concurrently during mkfs.
> >
> > Fix it by treating a zero return as an error (-ENODATA) so the
> > caller fails cleanly instead of hanging indefinitely.
> >
> > Also fix the long-standing 'pading' -> 'padding' typo in the
> > short-read diagnostic message of erofs_dev_read().
> >
> > Signed-off-by: Ajay Rajera <newajay.11r at gmail.com>
> > ---
> >  lib/io.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/io.c b/lib/io.c
> > index 0c5eb2c..583f52d 100644
> > --- a/lib/io.c
> > +++ b/lib/io.c
> > @@ -430,7 +430,7 @@ ssize_t erofs_dev_read(struct erofs_sb_info *sbi, int device_id,
> >       if (read < 0)
> >               return read;
> >       if (read < len) {
> > -             erofs_info("reach EOF of device @ %llu, pading with zeroes",
> > +             erofs_info("reach EOF of device @ %llu, padding with zeroes",
> >                          offset | 0ULL);
> >               memset(buf + read, 0, len - read);
> >       }
> > @@ -665,14 +665,15 @@ int erofs_io_xcopy(struct erofs_vfile *vout, off_t pos,
> >               int ret = min_t(unsigned int, len, sizeof(buf));
> >
> >               ret = erofs_io_read(vin, buf, ret);
> > -             if (ret < 0)
> > +             if (ret <= 0) {
> > +                     if (!ret)
> > +                             return -ENODATA;
> >                       return ret;
> > -             if (ret > 0) {
> > -                     ret = erofs_io_pwrite(vout, buf, pos, ret);
>
> Change looks good to me, just minor nits.
>
> Don't need the nested if and I think EIO would be better choice here
> based on your description. We would expect to see 0 from erofs_io_read
> when there is an issue with our input file, mostly caused by IO issues.
>
> if (ret < 0)
>         return ret;
> else if (!ret)
>         return -EIO;
>
> ret = erofs_io_pwrite(vout, buf, pos, ret);
>
> Regards,
> Lucas Karpinski
>
>
>
>