diff options
-rw-r--r-- | src/ChangeLog | 13 | ||||
-rw-r--r-- | src/filelock.c | 6 | ||||
-rw-r--r-- | src/image.c | 35 | ||||
-rw-r--r-- | src/process.c | 22 | ||||
-rw-r--r-- | src/xfaces.c | 20 |
5 files changed, 61 insertions, 35 deletions
diff --git a/src/ChangeLog b/src/ChangeLog index 73e24bcf82..a63e441dcb 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,18 @@ 2013-07-19 Paul Eggert <[email protected]> + 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. diff --git a/src/filelock.c b/src/filelock.c index fefd14b3a9..b9c991e4ba 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -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 diff --git a/src/image.c b/src/image.c index 95d385dc9e..1e3944ac1a 100644 --- a/src/image.c +++ b/src/image.c @@ -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; } } diff --git a/src/process.c b/src/process.c index 7c63964aee..f4ae662468 100644 --- a/src/process.c +++ b/src/process.c @@ -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); diff --git a/src/xfaces.c b/src/xfaces.c index d35851220b..f647ff2e20 100644 --- a/src/xfaces.c +++ b/src/xfaces.c @@ -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 |