[ping5][PATCH][BZ15362] Fix fwrite() reading beyond end of buffer in error path

[Eric Biggers <ebiggers3 at gmail dot com>]

Pinging my patch for BZ15362 yet again.

Patch included, but it is the same as posted at
https://sourceware.org/ml/libc-alpha/2013-04/msg00566.html except rebased on
master and retested, and ChangeLog date updated.

This patch was already reviewed and pinged a while back by Siddhesh Poyarekar
(see https://sourceware.org/ml/libc-alpha/2013-06/msg00625.html), but no one
else reviewed it.

Note that I classified this as a critical bug when I opened it since it involves
an invalid memory access being made.  Even worse, invalid/unexpected data may be
passed to the write() system call which could constitute an information
disclosure vulnerability, although this can only occur following a write error
on the same file descriptor.

-----

Partially revert commits 2b766585f9b4ffabeef2f36200c275976b93f2c7 and 
de2fd463b1c0310d75084b6d774fb974075a4ad9, which were intended to fix BZ#11741 
but caused another, likely worse bug, namely that fwrite() and fputs() could, 
in an error path, read data beyond the end of the specified buffer, and 
potentially even write this data to the file. 

Fix BZ#11741 properly by checking the return value from _IO_padn() in 
stdio-common/vfprintf.c. 

diff --git a/ChangeLog b/ChangeLog
index 8f264da..952fcff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2013-09-12  Eric Biggers  <ebiggers3@gmail.com>
+
+	[BZ #15362]
+	* libio/fileops.c:  Revert problematic fixes for [BZ #11741]
+	* libio/iofwrite.c:  Likewise.
+	* libio/iofwrite_u.c:  Likewise.
+	* libio/iopadn.c:  Likewise.
+	* libio/iowpadn.c:  Likewise.
+	* stdio-common/vfprintf.c:  Fix [BZ #11741] properly by checking whether
+	_IO_padn() returned the full count written.
+
 2013-09-11  Jia Liu  <proljc@gmail.com>
 
 	* sunrpc/rpc/types.h [__APPLE_CC__]: Define __u_char_defined and
diff --git a/libio/fileops.c b/libio/fileops.c
index e92f85b..c58e860 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -1245,13 +1245,12 @@ _IO_new_file_write (f, data, n)
      _IO_ssize_t n;
 {
   _IO_ssize_t to_do = n;
-  _IO_ssize_t count = 0;
   while (to_do > 0)
     {
-      count = (__builtin_expect (f->_flags2
-				 & _IO_FLAGS2_NOTCANCEL, 0)
-	       ? write_not_cancel (f->_fileno, data, to_do)
-	       : write (f->_fileno, data, to_do));
+      _IO_ssize_t count = (__builtin_expect (f->_flags2
+					     & _IO_FLAGS2_NOTCANCEL, 0)
+			   ? write_not_cancel (f->_fileno, data, to_do)
+			   : write (f->_fileno, data, to_do));
       if (count < 0)
 	{
 	  f->_flags |= _IO_ERR_SEEN;
@@ -1263,7 +1262,7 @@ _IO_new_file_write (f, data, n)
   n -= to_do;
   if (f->_offset >= 0)
     f->_offset += n;
-  return count < 0 ? count : n;
+  return n;
 }
 
 _IO_size_t
@@ -1323,13 +1322,11 @@ _IO_new_file_xsputn (f, data, n)
       _IO_size_t block_size, do_write;
       /* Next flush the (full) buffer. */
       if (_IO_OVERFLOW (f, EOF) == EOF)
-	/* If nothing else has to be written or nothing has been written, we
-	   must not signal the caller that the call was even partially
-	   successful.  */
-	return (to_do == 0 || to_do == n) ? EOF : n - to_do;
+	/* If nothing else has to be written we must not signal the
+	   caller that everything has been written.  */
+	return to_do == 0 ? EOF : n - to_do;
 
-      /* Try to maintain alignment: write a whole number of blocks.
-	 dont_write is what gets left over. */
+      /* Try to maintain alignment: write a whole number of blocks.  */
       block_size = f->_IO_buf_end - f->_IO_buf_base;
       do_write = to_do - (block_size >= 128 ? to_do % block_size : 0);
 
diff --git a/libio/iofwrite.c b/libio/iofwrite.c
index 81596a6..66542ea 100644
--- a/libio/iofwrite.c
+++ b/libio/iofwrite.c
@@ -42,12 +42,12 @@ _IO_fwrite (buf, size, count, fp)
   if (_IO_vtable_offset (fp) != 0 || _IO_fwide (fp, -1) == -1)
     written = _IO_sputn (fp, (const char *) buf, request);
   _IO_release_lock (fp);
-  /* We are guaranteed to have written all of the input, none of it, or
-     some of it.  */
-  if (written == request)
+  /* We have written all of the input in case the return value indicates
+     this or EOF is returned.  The latter is a special case where we
+     simply did not manage to flush the buffer.  But the data is in the
+     buffer and therefore written as far as fwrite is concerned.  */
+  if (written == request || written == EOF)
     return count;
-  else if (written == EOF)
-    return 0;
   else
     return written / size;
 }
diff --git a/libio/iofwrite_u.c b/libio/iofwrite_u.c
index 4a9d6ca..18dc6d0 100644
--- a/libio/iofwrite_u.c
+++ b/libio/iofwrite_u.c
@@ -44,12 +44,12 @@ fwrite_unlocked (buf, size, count, fp)
   if (_IO_fwide (fp, -1) == -1)
     {
       written = _IO_sputn (fp, (const char *) buf, request);
-      /* We are guaranteed to have written all of the input, none of it, or
-	 some of it.  */
-      if (written == request)
+      /* We have written all of the input in case the return value indicates
+	 this or EOF is returned.  The latter is a special case where we
+	 simply did not manage to flush the buffer.  But the data is in the
+	 buffer and therefore written as far as fwrite is concerned.  */
+      if (written == request || written == EOF)
 	return count;
-      else if (written == EOF)
-	return 0;
     }
 
   return written / size;
diff --git a/libio/iopadn.c b/libio/iopadn.c
index cc93c0f..5ebbcf4 100644
--- a/libio/iopadn.c
+++ b/libio/iopadn.c
@@ -59,7 +59,7 @@ _IO_padn (fp, pad, count)
       w = _IO_sputn (fp, padptr, PADSIZE);
       written += w;
       if (w != PADSIZE)
-	return w == EOF ? w : written;
+	return written;
     }
 
   if (i > 0)
diff --git a/libio/iowpadn.c b/libio/iowpadn.c
index d94db71..5600f37 100644
--- a/libio/iowpadn.c
+++ b/libio/iowpadn.c
@@ -65,7 +65,7 @@ _IO_wpadn (fp, pad, count)
       w = _IO_sputn (fp, (char *) padptr, PADSIZE);
       written += w;
       if (w != PADSIZE)
-	return w == EOF ? w : written;
+	return written;
     }
 
   if (i > 0)
diff --git a/stdio-common/vfprintf.c b/stdio-common/vfprintf.c
index fb22f69..8cd7a85 100644
--- a/stdio-common/vfprintf.c
+++ b/stdio-common/vfprintf.c
@@ -90,13 +90,13 @@
   do {									      \
     if (width > 0)							      \
       {									      \
-	unsigned int d = _IO_padn (s, (Padchar), width);		      \
-	if (__glibc_unlikely (d == EOF))				      \
+	_IO_ssize_t written = _IO_padn (s, (Padchar), width);		      \
+	if (__glibc_unlikely (written != width))			      \
 	  {								      \
 	    done = -1;							      \
 	    goto all_done;						      \
 	  }								      \
-	done_add (d);							      \
+	done_add (written);						      \
       }									      \
   } while (0)
 # define PUTC(C, F)	_IO_putc_unlocked (C, F)
@@ -119,13 +119,13 @@
   do {									      \
     if (width > 0)							      \
       {									      \
-	unsigned int d = _IO_wpadn (s, (Padchar), width);		      \
-	if (__glibc_unlikely (d == EOF))				      \
+	_IO_ssize_t written = _IO_wpadn (s, (Padchar), width);		      \
+	if (__glibc_unlikely (written != width))			      \
 	  {								      \
 	    done = -1;							      \
 	    goto all_done;						      \
 	  }								      \
-	done_add (d);							      \
+	done_add (written);						      \
       }									      \
   } while (0)
 # define PUTC(C, F)	_IO_putwc_unlocked (C, F)