* [musl] Bug in src/stdio/fread.c
@ 2021-07-10 13:10 jason
2021-07-10 16:50 ` Rich Felker
0 siblings, 1 reply; 4+ messages in thread
From: jason @ 2021-07-10 13:10 UTC (permalink / raw)
To: musl; +Cc: jason
If you look at the code:
size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
{
unsigned char *dest = destv;
size_t len = size*nmemb, l = len, k;
if (!size) nmemb = 0;
FLOCK(f);
f->mode |= f->mode-1;
if (f->rpos != f->rend) {
/* First exhaust the buffer. */
k = MIN(f->rend - f->rpos, l);
memcpy(dest, f->rpos, k);
f->rpos += k;
dest += k;
l -= k;
}
/* Read the remainder directly */
for (; l; l-=k, dest+=k) {
k = __toread(f) ? 0 : f->read(f, dest, l);
if (!k) {
FUNLOCK(f);
return (len-l)/size;
}
}
FUNLOCK(f);
return nmemb;
}
Consider what happens when f->rpos == f->rend: k is used uninitialized.
My suggested fix is:
- if (f->rpos != f->rend) {
+ k = f->rend - f->rpos;
+ if (!k) {
/* First exhaust the buffer. */
- k = MIN(f->rend - f->rpos, l);
+ k = MIN(k, l);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Bug in src/stdio/fread.c
2021-07-10 13:10 [musl] Bug in src/stdio/fread.c jason
@ 2021-07-10 16:50 ` Rich Felker
2021-07-10 17:13 ` David Edelsohn
0 siblings, 1 reply; 4+ messages in thread
From: Rich Felker @ 2021-07-10 16:50 UTC (permalink / raw)
To: jason; +Cc: musl
On Sat, Jul 10, 2021 at 03:10:26PM +0200, jason wrote:
> If you look at the code:
>
> size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
> {
> unsigned char *dest = destv;
> size_t len = size*nmemb, l = len, k;
> if (!size) nmemb = 0;
>
> FLOCK(f);
>
> f->mode |= f->mode-1;
>
> if (f->rpos != f->rend) {
> /* First exhaust the buffer. */
> k = MIN(f->rend - f->rpos, l);
> memcpy(dest, f->rpos, k);
> f->rpos += k;
> dest += k;
> l -= k;
> }
>
> /* Read the remainder directly */
> for (; l; l-=k, dest+=k) {
> k = __toread(f) ? 0 : f->read(f, dest, l);
> if (!k) {
> FUNLOCK(f);
> return (len-l)/size;
> }
> }
>
> FUNLOCK(f);
> return nmemb;
> }
>
> Consider what happens when f->rpos == f->rend: k is used uninitialized.
At which line?
Rich
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Bug in src/stdio/fread.c
2021-07-10 16:50 ` Rich Felker
@ 2021-07-10 17:13 ` David Edelsohn
2021-07-10 17:17 ` David Edelsohn
0 siblings, 1 reply; 4+ messages in thread
From: David Edelsohn @ 2021-07-10 17:13 UTC (permalink / raw)
To: musl; +Cc: jason
On Sat, Jul 10, 2021 at 12:51 PM Rich Felker <dalias@libc.org> wrote:
>
> On Sat, Jul 10, 2021 at 03:10:26PM +0200, jason wrote:
> > If you look at the code:
> >
> > size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
> > {
> > unsigned char *dest = destv;
k declared but not initialized.
> > size_t len = size*nmemb, l = len, k;
> > if (!size) nmemb = 0;
> >
> > FLOCK(f);
> >
> > f->mode |= f->mode-1;
> >
> > if (f->rpos != f->rend) {
> > /* First exhaust the buffer. */
k set to value.
> > k = MIN(f->rend - f->rpos, l);
> > memcpy(dest, f->rpos, k);
> > f->rpos += k;
> > dest += k;
> > l -= k;
> > }
> >
> > /* Read the remainder directly */
USE of k. If f->rpos == f->rend, k was never set before use for the
first iteration of the loop.
> > for (; l; l-=k, dest+=k) {
> > k = __toread(f) ? 0 : f->read(f, dest, l);
> > if (!k) {
> > FUNLOCK(f);
> > return (len-l)/size;
> > }
> > }
> >
> > FUNLOCK(f);
> > return nmemb;
> > }
> >
> > Consider what happens when f->rpos == f->rend: k is used uninitialized.
>
> At which line?
>
> Rich
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [musl] Bug in src/stdio/fread.c
2021-07-10 17:13 ` David Edelsohn
@ 2021-07-10 17:17 ` David Edelsohn
0 siblings, 0 replies; 4+ messages in thread
From: David Edelsohn @ 2021-07-10 17:17 UTC (permalink / raw)
To: musl; +Cc: jason
On Sat, Jul 10, 2021 at 1:13 PM David Edelsohn <dje.gcc@gmail.com> wrote:
>
> On Sat, Jul 10, 2021 at 12:51 PM Rich Felker <dalias@libc.org> wrote:
> >
> > On Sat, Jul 10, 2021 at 03:10:26PM +0200, jason wrote:
> > > If you look at the code:
> > >
> > > size_t fread(void *restrict destv, size_t size, size_t nmemb, FILE *restrict f)
> > > {
> > > unsigned char *dest = destv;
>
> k declared but not initialized.
>
> > > size_t len = size*nmemb, l = len, k;
> > > if (!size) nmemb = 0;
> > >
> > > FLOCK(f);
> > >
> > > f->mode |= f->mode-1;
> > >
> > > if (f->rpos != f->rend) {
> > > /* First exhaust the buffer. */
>
> k set to value.
>
> > > k = MIN(f->rend - f->rpos, l);
> > > memcpy(dest, f->rpos, k);
> > > f->rpos += k;
> > > dest += k;
> > > l -= k;
> > > }
> > >
> > > /* Read the remainder directly */
>
> USE of k. If f->rpos == f->rend, k was never set before use for the
> first iteration of the loop.
>
> > > for (; l; l-=k, dest+=k) {
> > > k = __toread(f) ? 0 : f->read(f, dest, l);
> > > if (!k) {
> > > FUNLOCK(f);
> > > return (len-l)/size;
> > > }
> > > }
> > >
> > > FUNLOCK(f);
> > > return nmemb;
> > > }
> > >
> > > Consider what happens when f->rpos == f->rend: k is used uninitialized.
> >
> > At which line?
Sorry, my mistake, k will be set in the loop before the iteration
expression is evaluated.
- David
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-10 17:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-10 13:10 [musl] Bug in src/stdio/fread.c jason
2021-07-10 16:50 ` Rich Felker
2021-07-10 17:13 ` David Edelsohn
2021-07-10 17:17 ` David Edelsohn
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).