libc/stdio: uninitialized buffer size in __swhatbuf()

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

libc/stdio: uninitialized buffer size in __swhatbuf()

Florin Malita
Scanning through the stdio sources I noticed a code path that could
leave size/*bufsize uninitialized in __smakebuf()/__swhatbuf():

http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/stdio/makebuf.c?rev=1.7&content-type=text/x-cvsweb-markup

void
__smakebuf(FILE *fp)
{
        void *p;
        int flags;
        size_t size;
        int couldbetty;

        if (fp->_flags & __SNBF) {
                fp->_bf._base = fp->_p = fp->_nbuf;
                fp->_bf._size = 1;
                return;
        }
        flags = __swhatbuf(fp, &size, &couldbetty);
        if ((p = malloc(size)) == NULL) {
...
...

int
__swhatbuf(FILE *fp, size_t *bufsize, int *couldbetty)
{
        struct stat st;

        if (fp->_file < 0 || fstat(fp->_file, &st) < 0) {
                *couldbetty = 0;
                *bufsize = BUFSIZ;
                return (__SNPT);
        }

        /* could be a tty iff it is a character device */
        *couldbetty = S_ISCHR(st.st_mode);
        if (st.st_blksize == 0) {
                *bufsize = BUFSIZ;
                return (__SNPT);
        }

        /*
         * Optimise fseek() only if it is a regular file.  (The test for
         * __sseek is mainly paranoia.)  It is safe to set _blksize
         * unconditionally; it will only be used if __SOPT is also set.
         */
        if ((fp->_flags & __SSTR) == 0) {
                *bufsize = st.st_blksize;
                fp->_blksize = st.st_blksize;
        }
        return ((st.st_mode & S_IFMT) == S_IFREG && fp->_seek == __sseek ?
            __SOPT : __SNPT);
}

If (fp->_flags & __SSTR) != 0 then *bufsize is not being initialized.
I'm not sure whether this condition can actually occur (if it cannot
then the conditional statement is redundant), but if it does
__smakebuf() will totally mess up the buffering.

Trivial fix follows.

Regards,
Florin


--- makebuf.c.orig 2005-12-27 22:46:45.000000000 -0500
+++ makebuf.c 2005-12-27 22:47:52.000000000 -0500
@@ -82,16 +82,16 @@
 {
  struct stat st;
 
+ *bufsize = BUFSIZ;
+
  if (fp->_file < 0 || fstat(fp->_file, &st) < 0) {
  *couldbetty = 0;
- *bufsize = BUFSIZ;
  return (__SNPT);
  }
 
  /* could be a tty iff it is a character device */
  *couldbetty = S_ISCHR(st.st_mode);
  if (st.st_blksize == 0) {
- *bufsize = BUFSIZ;
  return (__SNPT);
  }

Reply | Threaded
Open this post in threaded view
|

Re: libc/stdio: uninitialized buffer size in __swhatbuf()

Todd C. Miller
The "if ((fp->_flags & __SSTR) == 0)" was added by me in rev 1.3.
If __SSTR is set we are in sprintf/snprintf where file buffering
doesn't make sense.  However, I don't think that code path will
ever happen for sprintf/snprintf since fp->_bf._base cannot be NULL.
That part of rev 1.3 can probably be undone and, based on the
commit message, it was probably committed by mistake anyway.

 - todd