On Mon, Dec 28, 2020 at 5:24 PM Waldek Kozaczuk <jwkozac...@gmail.com>
wrote:

> How did you discover this bug?
>

I am trying to run Lua 5.4, and because of misconfiguration, I caused it to
read a file with an empty name (!)
and this resulted in read errors, and fread() just went into an endless
loop and I had to debug why....
Then I found the incorrect logic in fread(), and "git blame" told me the
logic is correct, if you change __stdio_read() accordingly - and it turns
out that's what they did in Musl.

I was too lazy to write a unit test that reproduces the bug. But I should!
It should be very easy (a simple fread() from a directory should result in
an endless loop) I will do it later.



> Sent from my iPhone
>
> > On Dec 28, 2020, at 08:54, Nadav Har'El <n...@scylladb.com> wrote:
> >
> > We have our own copy of __stdio_read.c because it was too complicated
> > to take Musl's as-is because of locking differences. However, there was
> > one recent change to that file we forgot to take:
> >
> > In commit f92804188 in Musl's repository, Rich Felker (in Feb. 2018),
> > changed fread() to assume that f->read() returns 0 on error, instead
> > of -1. The same patch fixes __stdio_read() to return 0, and we need
> > to do this as well.
> >
> > Without this patch, when the new Musl's fread() encounters an error
> (e.g.,
> > when you try to read from a directory), it will go into an infinite
> > loop - each iteration increases the remaining bytes (by 1) instead of
> > decreasing it by the number of read bytes as normal.
> >
> > Signed-off-by: Nadav Har'El <n...@scylladb.com>
> > ---
> > libc/stdio/__stdio_read.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libc/stdio/__stdio_read.c b/libc/stdio/__stdio_read.c
> > index 8fec9c35..30d7aac5 100644
> > --- a/libc/stdio/__stdio_read.c
> > +++ b/libc/stdio/__stdio_read.c
> > @@ -30,7 +30,7 @@ size_t __stdio_read(FILE *f, unsigned char *buf,
> size_t len)
> >    if (cnt <= 0) {
> >        f->flags |= F_EOF ^ ((F_ERR^F_EOF) & cnt);
> >        f->rpos = f->rend = 0;
> > -        return cnt;
> > +        return 0;
> >    }
> >    if (cnt <= len) return cnt;
> >    cnt -= len;
> > --
> > 2.29.2
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "OSv Development" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to osv-dev+unsubscr...@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20201228135352.3246222-1-nyh%40scylladb.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjsqcJ%3Dw9BC3bcCt3ho7-yxd_9YVOOsoacD8DrEDhzYfHQ%40mail.gmail.com.

Reply via email to