]> git.eshelyaron.com Git - emacs.git/commitdiff
Fix some minor file descriptor leaks and related glitches.
authorPaul Eggert <eggert@cs.ucla.edu>
Fri, 19 Jul 2013 18:09:23 +0000 (11:09 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Fri, 19 Jul 2013 18:09:23 +0000 (11:09 -0700)
* filelock.c (create_lock_file) [!O_CLOEXEC]: Use fcntl with FD_CLOEXEC.
(create_lock_file): Use write, not emacs_write.
* image.c (slurp_file, png_load_body):
* process.c (Fnetwork_interface_list, Fnetwork_interface_info)
(server_accept_connection):
Don't leak an fd on memory allocation failure.
* image.c (slurp_file): Add a cheap heuristic for growing files.
* xfaces.c (Fx_load_color_file): Block input around the fopen too,
as that's what the other routines do.  Maybe input need not be
blocked at all, but it's better to be consistent.
Avoid undefined behavior when strlen is zero.

src/ChangeLog
src/filelock.c
src/image.c
src/process.c
src/xfaces.c

index 73e24bcf8291cfcae1ca8b7faeaa9b71adc0c342..a63e441dcb2d02f0e37031a6c278c38422aaf7ad 100644 (file)
@@ -1,5 +1,18 @@
 2013-07-19  Paul Eggert  <eggert@cs.ucla.edu>
 
+       Fix some minor file descriptor leaks and related glitches.
+       * filelock.c (create_lock_file) [!O_CLOEXEC]: Use fcntl with FD_CLOEXEC.
+       (create_lock_file): Use write, not emacs_write.
+       * image.c (slurp_file, png_load_body):
+       * process.c (Fnetwork_interface_list, Fnetwork_interface_info)
+       (server_accept_connection):
+       Don't leak an fd on memory allocation failure.
+       * image.c (slurp_file): Add a cheap heuristic for growing files.
+       * xfaces.c (Fx_load_color_file): Block input around the fopen too,
+       as that's what the other routines do.  Maybe input need not be
+       blocked at all, but it's better to be consistent.
+       Avoid undefined behavior when strlen is zero.
+
        * alloc.c (staticpro): Avoid buffer overrun on repeated calls.
        (NSTATICS): Now a constant; doesn't need to be a macro.
 
index fefd14b3a92d5a1e5bd7d19048c3b19ec17db764..b9c991e4bafea61c66a449935780aab3a7dee4e1 100644 (file)
@@ -430,12 +430,14 @@ create_lock_file (char *lfname, char *lock_info_str, bool force)
       else
        {
          ptrdiff_t lock_info_len;
-#if ! HAVE_MKOSTEMP
+#if ! (HAVE_MKOSTEMP && O_CLOEXEC)
          fcntl (fd, F_SETFD, FD_CLOEXEC);
 #endif
          lock_info_len = strlen (lock_info_str);
          err = 0;
-         if (emacs_write (fd, lock_info_str, lock_info_len) != lock_info_len
+         /* Use 'write', not 'emacs_write', as garbage collection
+            might signal an error, which would leak FD.  */
+         if (write (fd, lock_info_str, lock_info_len) != lock_info_len
              || fchmod (fd, S_IRUSR | S_IRGRP | S_IROTH) != 0)
            err = errno;
          /* There is no need to call fsync here, as the contents of
index 95d385dc9e2a0d22b55965f670e7070b203a4154..1e3944ac1a1d9a028bfbb17da605a9d74cedbfc7 100644 (file)
@@ -2276,23 +2276,28 @@ slurp_file (char *file, ptrdiff_t *size)
   unsigned char *buf = NULL;
   struct stat st;
 
-  if (fp && fstat (fileno (fp), &st) == 0
-      && 0 <= st.st_size && st.st_size <= min (PTRDIFF_MAX, SIZE_MAX)
-      && (buf = xmalloc (st.st_size),
-         fread (buf, 1, st.st_size, fp) == st.st_size))
-    {
-      *size = st.st_size;
-      fclose (fp);
-    }
-  else
+  if (fp)
     {
-      if (fp)
-       fclose (fp);
-      if (buf)
+      ptrdiff_t count = SPECPDL_INDEX ();
+      record_unwind_protect_ptr (fclose_unwind, fp);
+
+      if (fstat (fileno (fp), &st) == 0
+         && 0 <= st.st_size && st.st_size < min (PTRDIFF_MAX, SIZE_MAX))
        {
-         xfree (buf);
-         buf = NULL;
+         /* Report an error if we read past the purported EOF.
+            This can happen if the file grows as we read it.  */
+         ptrdiff_t buflen = st.st_size;
+         buf = xmalloc (buflen + 1);
+         if (fread (buf, 1, buflen + 1, fp) == buflen)
+           *size = buflen;
+         else
+           {
+             xfree (buf);
+             buf = NULL;
+           }
        }
+
+      unbind_to (count, Qnil);
     }
 
   return buf;
@@ -5732,8 +5737,8 @@ png_load_body (struct frame *f, struct image *img, struct png_load_context *c)
       if (fread (sig, 1, sizeof sig, fp) != sizeof sig
          || fn_png_sig_cmp (sig, 0, sizeof sig))
        {
-         image_error ("Not a PNG file: `%s'", file, Qnil);
          fclose (fp);
+         image_error ("Not a PNG file: `%s'", file, Qnil);
          return 0;
        }
     }
index 7c63964aee60291eb667e21debd6d8e0457489c6..f4ae662468b8f98810ddb637b1817c8e62e43d0b 100644 (file)
@@ -3526,10 +3526,13 @@ format; see the description of ADDRESS in `make-network-process'.  */)
   ptrdiff_t buf_size = 512;
   int s;
   Lisp_Object res;
+  ptrdiff_t count;
 
   s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
 
   do
     {
@@ -3545,9 +3548,7 @@ format; see the description of ADDRESS in `make-network-process'.  */)
     }
   while (ifconf.ifc_len == buf_size);
 
-  emacs_close (s);
-
-  res = Qnil;
+  res = unbind_to (count, Qnil);
   ifreq = ifconf.ifc_req;
   while ((char *) ifreq < (char *) ifconf.ifc_req + ifconf.ifc_len)
     {
@@ -3672,6 +3673,7 @@ FLAGS is the current flags of the interface.  */)
   Lisp_Object elt;
   int s;
   bool any = 0;
+  ptrdiff_t count;
 #if (! (defined SIOCGIFHWADDR && defined HAVE_STRUCT_IFREQ_IFR_HWADDR) \
      && defined HAVE_GETIFADDRS && defined LLADDR)
   struct ifaddrs *ifap;
@@ -3686,6 +3688,8 @@ FLAGS is the current flags of the interface.  */)
   s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0);
   if (s < 0)
     return Qnil;
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
 
   elt = Qnil;
 #if defined (SIOCGIFFLAGS) && defined (HAVE_STRUCT_IFREQ_IFR_FLAGS)
@@ -3802,9 +3806,7 @@ FLAGS is the current flags of the interface.  */)
 #endif
   res = Fcons (elt, res);
 
-  emacs_close (s);
-
-  return any ? res : Qnil;
+  return unbind_to (count, any ? res : Qnil);
 }
 #endif
 #endif /* defined (HAVE_NET_IF_H) */
@@ -3978,6 +3980,7 @@ server_accept_connection (Lisp_Object server, int channel)
 #endif
   } saddr;
   socklen_t len = sizeof saddr;
+  ptrdiff_t count;
 
   s = accept4 (channel, &saddr.sa, &len, SOCK_CLOEXEC);
 
@@ -4000,6 +4003,9 @@ server_accept_connection (Lisp_Object server, int channel)
       return;
     }
 
+  count = SPECPDL_INDEX ();
+  record_unwind_protect_int (close_file_unwind, s);
+
   connect_counter++;
 
   /* Setup a new process to handle the connection.  */
@@ -4116,6 +4122,10 @@ server_accept_connection (Lisp_Object server, int channel)
   pset_filter (p, ps->filter);
   pset_command (p, Qnil);
   p->pid = 0;
+
+  /* Discard the unwind protect for closing S.  */
+  specpdl_ptr = specpdl + count;
+
   p->infd  = s;
   p->outfd = s;
   pset_status (p, Qrun);
index d35851220b022c153747147b4b685d3cc90363bd..f647ff2e20983e9af42ce797becab4611a0dbe34 100644 (file)
@@ -6283,6 +6283,7 @@ where R,G,B are numbers between 0 and 255 and name is an arbitrary string.  */)
   CHECK_STRING (filename);
   abspath = Fexpand_file_name (filename, Qnil);
 
+  block_input ();
   fp = emacs_fopen (SSDATA (abspath), "rt");
   if (fp)
     {
@@ -6290,29 +6291,24 @@ where R,G,B are numbers between 0 and 255 and name is an arbitrary string.  */)
       int red, green, blue;
       int num;
 
-      block_input ();
-
       while (fgets (buf, sizeof (buf), fp) != NULL) {
        if (sscanf (buf, "%u %u %u %n", &red, &green, &blue, &num) == 3)
          {
-           char *name = buf + num;
-           num = strlen (name) - 1;
-           if (num >= 0 && name[num] == '\n')
-             name[num] = 0;
-           cmap = Fcons (Fcons (build_string (name),
 #ifdef HAVE_NTGUI
-                                make_number (RGB (red, green, blue))),
+           int color = RGB (red, green, blue);
 #else
-                                make_number ((red << 16) | (green << 8) | blue)),
+           int color = (red << 16) | (green << 8) | blue;
 #endif
+           char *name = buf + num;
+           ptrdiff_t len = strlen (name);
+           len -= 0 < len && name[len - 1] == '\n';
+           cmap = Fcons (Fcons (make_string (name, len), make_number (color)),
                          cmap);
          }
       }
       fclose (fp);
-
-      unblock_input ();
     }
-
+  unblock_input ();
   return cmap;
 }
 #endif