diff mbox

Fix BZ #18757 (fmemopen doesn't fail with invalid modes)

Message ID CALoOobOh380-KyFdrwO5kEOt8ks0HMQoG7_VNYvXZaJvcqABFw@mail.gmail.com
State New
Headers show

Commit Message

Paul Pluzhnikov Aug. 10, 2015, 5 a.m. UTC 
On Sun, Aug 9, 2015 at 9:15 PM, Mike Frysinger <vapier@gentoo.org> wrote:

> i think you also need to free c->buffer when c->mybuffer is true

Right.


2015-08-09  Paul Pluzhnikov  <ppluzhnikov@google.com>

        [BZ #18757]
        * libio/fmemopen.c (__fmemopen): Set EINVAL and don't leak memory.
        * libio/oldfmemopen.c (__old_fmemopen): Likewise.
        * libio/test-fmemopen.c (do_bz18757): New test.

Comments

Mike Frysinger Aug. 10, 2015, 6:26 a.m. UTC | #1 
On 09 Aug 2015 22:00, Paul Pluzhnikov wrote:
> -  return _IO_fopencookie (c, mode, iof);
> +  ret = _IO_fopencookie (c, mode, iof);
> +
> +  if (__glibc_unlikely (ret == NULL))
> +    {
> +      /* BZ #18757 -- set EINVAL.  */
> +      __set_errno (EINVAL);

i'm not sure normalizing all errors into EINVAL is correct.  you might get back
NULL if malloc failed in which case it'd be ENOMEM.  i think we want to plumb
errno setting down further where the actual errors are flagged and leave this
func alone.
-mike
diff mbox

Patch

diff --git a/libio/fmemopen.c b/libio/fmemopen.c
index 3ab3e8d..ad64f04 100644
--- a/libio/fmemopen.c
+++ b/libio/fmemopen.c
@@ -149,6 +149,7 @@  __fmemopen (void *buf, size_t len, const char *mode)
 {
   cookie_io_functions_t iof;
   fmemopen_cookie_t *c;
+  FILE *ret;
 
   c = (fmemopen_cookie_t *) calloc (sizeof (fmemopen_cookie_t), 1);
   if (c == NULL)
@@ -209,7 +210,20 @@  __fmemopen (void *buf, size_t len, const char *mode)
   iof.seek = fmemopen_seek;
   iof.close = fmemopen_close;
 
-  return _IO_fopencookie (c, mode, iof);
+  ret = _IO_fopencookie (c, mode, iof);
+
+  if (__glibc_unlikely (ret == NULL))
+    {
+      /* BZ #18757 -- set EINVAL.  */
+      __set_errno (EINVAL);
+
+      if (c->mybuffer)
+	free (c->buffer);
+
+      free (c);
+    }
+
+  return ret;
 }
 libc_hidden_def (__fmemopen)
 versioned_symbol (libc, __fmemopen, fmemopen, GLIBC_2_22);
diff --git a/libio/oldfmemopen.c b/libio/oldfmemopen.c
index 8e35672..1b331a5 100644
--- a/libio/oldfmemopen.c
+++ b/libio/oldfmemopen.c
@@ -204,6 +204,7 @@  __old_fmemopen (void *buf, size_t len, const char *mode)
 {
   cookie_io_functions_t iof;
   fmemopen_cookie_t *c;
+  FILE *ret;
 
   if (__glibc_unlikely (len == 0))
     {
@@ -259,7 +260,19 @@  __old_fmemopen (void *buf, size_t len, const char *mode)
   iof.seek = fmemopen_seek;
   iof.close = fmemopen_close;
 
-  return _IO_fopencookie (c, mode, iof);
+  ret = _IO_fopencookie (c, mode, iof);
+  if (__glibc_unlikely (ret == NULL))
+    {
+      /* BZ 18757 -- set EINVAL.  */
+      __set_errno (EINVAL);
+
+      if (c->mybuffer)
+	free (c->buffer);
+
+      free (c);
+    }
+
+  return ret;
 }
 compat_symbol (libc, __old_fmemopen, fmemopen, GLIBC_2_2);
 #endif
diff --git a/libio/test-fmemopen.c b/libio/test-fmemopen.c
index 63ca89f..4d15b36 100644
--- a/libio/test-fmemopen.c
+++ b/libio/test-fmemopen.c
@@ -24,6 +24,28 @@  static char buffer[] = "foobar";
 #include <errno.h>
 
 static int
+do_bz18757 (void)
+{
+  char c = 0;
+  FILE *stream;
+
+  errno = 0;
+  stream = fmemopen (&c, 1, "?");
+  if (stream == NULL)
+    {
+      if (errno == EINVAL)
+        return 0;
+
+      printf ("FAIL: errno = %i, but wanted EINVAL (%i)\n", errno, EINVAL);
+      return 1;
+    }
+
+  printf ("FAIL: stream = %p, but wanted NULL\n", stream);
+  fclose (stream);
+  return 2;
+}
+
+static int
 do_test (void)
 {
   int ch;
@@ -44,7 +66,7 @@  do_test (void)
 
   fclose (stream);
 
-  return ret;
+  return ret + do_bz18757 ();
 }
 
 #define TEST_FUNCTION do_test ()