diff mbox

Fix BZ #17916 fopen unbounded stack usage for ccs= modes

Message ID CALoOobPBCpZwMyF5F_4XzNJKKi64xMGtWgBXO_iDU_HpZ6+V9g@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Feb. 22, 2015, 8:48 p.m. UTC 
Greetings,

Attached patch fixes BZ #17916 fopen unbounded stack usage for ccs= modes
Tested on Linux/x86_64, no failures.

2015-02-22  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #17916]
        * libio/fileops.c (_IO_new_file_fopen): Limit stack use


Thanks,

Comments

Florian Weimer Feb. 22, 2015, 8:59 p.m. UTC | #1 
* Paul Pluzhnikov:

> Attached patch fixes BZ #17916 fopen unbounded stack usage for ccs= modes
> Tested on Linux/x86_64, no failures.

This should have a test case.

There is a missing NULL check on the malloc result.  I don't think the
alloca optimization makes sense because this functionality is used so
rarely, and the function allocates on the heap anyway.  Just use
malloc unconditionally; it will result in smaller code.
diff mbox

Patch

diff --git a/NEWS b/NEWS
index 28ef45d..660fbe0 100644
--- a/NEWS
+++ b/NEWS
@@ -10,8 +10,8 @@  Version 2.22
 * The following bugs are resolved with this release:
 
   4719, 13064, 14094, 15319, 15467, 15790, 16560, 17269, 17569, 17588,
-  17792, 17912, 17932, 17944, 17949, 17964, 17965, 17967, 17969, 17978,
-  17987, 17991, 17996, 17998, 17999.
+  17792, 17912, 17916, 17932, 17944, 17949, 17964, 17965, 17967, 17969,
+  17978, 17987, 17991, 17996, 17998, 17999.
 
 * Character encoding and ctype tables were updated to Unicode 7.0.0, using
   new generator scripts contributed by Pravin Satpute and Mike FABIAN (Red
diff --git a/libio/fileops.c b/libio/fileops.c
index 297b478..cf61996 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -353,7 +353,14 @@  _IO_new_file_fopen (_IO_FILE *fp, const char *filename, const char *mode,
 	  struct gconv_fcts fcts;
 	  struct _IO_codecvt *cc;
 	  char *endp = __strchrnul (cs + 5, ',');
-	  char ccs[endp - (cs + 5) + 3];
+	  const size_t ccs_size = endp - (cs + 5) + 3;
+	  const bool alloca_ccs = __libc_use_alloca (ccs_size);
+	  char *ccs;
+
+	  if (__glibc_likely (alloca_ccs))
+	    ccs = alloca (ccs_size);
+	  else
+	    ccs = malloc (ccs_size);
 
 	  *((char *) __mempcpy (ccs, cs + 5, endp - (cs + 5))) = '\0';
 	  strip (ccs, ccs);
@@ -365,10 +372,15 @@  _IO_new_file_fopen (_IO_FILE *fp, const char *filename, const char *mode,
 		 This means we cannot proceed since the user explicitly asked
 		 for these.  */
 	      (void) _IO_file_close_it (fp);
+	      if (! __glibc_likely (alloca_ccs))
+		free (ccs);
 	      __set_errno (EINVAL);
 	      return NULL;
 	    }
 
+	  if (! __glibc_likely (alloca_ccs))
+	    free (ccs);
+
 	  assert (fcts.towc_nsteps == 1);
 	  assert (fcts.tomb_nsteps == 1);