This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix iogetdelim.c (latent) integer overflow (bug 9914)


Bug 9914 reports libio/iogetdelim.c wrongly trying to detect integer
overflow after the event by seeing whether the result of an addition
of positive values is negative, so resulting in the check potentially
being optimized away.  This patch fixes this in the way described in a
comment in the bug.  No testcase is added; I believe the bug is in
fact latent (that the compiler won't be able to deduce that the

  len = fp->_IO_read_end - fp->_IO_read_ptr;
  if (len <= 0)
    {
      if (__underflow (fp) == EOF)
        {
          result = -1;
          goto unlock_return;
        }
      len = fp->_IO_read_end - fp->_IO_read_ptr;
    }

code does not result in a negative value of len that is still live at
this point).

(Note: I'm assuming that _IO_ssize_t always has the same limits as
ssize_t.  _IO_ssize_t is defined to _G_ssize_t and it's indeed the
case that all versions of _G_config.h do define that to __ssize_t.
Given that libio is nowadays only used as part of glibc - it hasn't
been used in libstdc++ for many years - I'm doubtful that most of the
_G_config.h abstractions make sense any more and think some of the
internal layers of types and macros in libio could reasonably be
cleaned up.  The only difference between the two versions of
_G_config.h that are actually used by current ports -
sysdeps/gnu/_G_config.h and sysdeps/mach/hurd/_G_config.h - is that
Hurd doesn't have mremap.)

Tested x86_64.

2012-09-03  Joseph Myers  <joseph@codesourcery.com>

	[BZ #9914]
	* libio/iogetdelim.c: Include <limits.h>.
	(_IO_getdelim): Avoid integer overflow in testing whether cur_len
	+ len + 1 would overflow.

diff --git a/libio/iogetdelim.c b/libio/iogetdelim.c
index 405b65f..bf4b0f7 100644
--- a/libio/iogetdelim.c
+++ b/libio/iogetdelim.c
@@ -29,6 +29,7 @@
 #include "libioP.h"
 #include <string.h>
 #include <errno.h>
+#include <limits.h>
 
 /* Read up to (and including) a TERMINATOR from FP into *LINEPTR
    (and null-terminate it).  *LINEPTR is a pointer returned from malloc (or
@@ -89,7 +90,7 @@ _IO_getdelim (lineptr, n, delimiter, fp)
       t = (char *) memchr ((void *) fp->_IO_read_ptr, delimiter, len);
       if (t != NULL)
 	len = (t - fp->_IO_read_ptr) + 1;
-      if (__builtin_expect (cur_len + len + 1 < 0, 0))
+      if (__builtin_expect (len >= SSIZE_MAX - cur_len, 0))
 	{
 	  __set_errno (EOVERFLOW);
 	  result = -1;

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]