diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/ChangeLog | 38 | ||||
| -rw-r--r-- | src/alloc.c | 2 | ||||
| -rw-r--r-- | src/callproc.c | 21 | ||||
| -rw-r--r-- | src/emacs.c | 7 | ||||
| -rw-r--r-- | src/filelock.c | 15 | ||||
| -rw-r--r-- | src/lisp.h | 2 | ||||
| -rw-r--r-- | src/lread.c | 33 | ||||
| -rw-r--r-- | src/nsterm.m | 2 | ||||
| -rw-r--r-- | src/process.c | 91 | ||||
| -rw-r--r-- | src/sysdep.c | 34 | ||||
| -rw-r--r-- | src/w32.c | 13 | ||||
| -rw-r--r-- | src/w32.h | 2 |
12 files changed, 140 insertions, 120 deletions
diff --git a/src/ChangeLog b/src/ChangeLog index 069d39ce6be..07285d564b2 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,41 @@ +2013-07-07 Paul Eggert <eggert@cs.ucla.edu> + + Make file descriptors close-on-exec when possible (Bug#14803). + This simplifies Emacs a bit, since it no longer needs to worry + about closing file descriptors by hand in some cases. + It also fixes some unlikely races. Not all such races, as + libraries often open files internally without setting + close-on-exec, but it's an improvement. + * alloc.c (valid_pointer_p) [!WINDOWSNT]: + * callproc.c (Fcall_process) [!MSDOS]: + * emacs.c (main) [!DOS_NT]: + * nsterm.m (ns_term_init): + * process.c (create_process): + Use 'pipe2' with O_CLOEXEC instead of 'pipe'. + * emacs.c (Fcall_process_region) [HAVE_MKOSTEMP]: + * filelock.c (create_lock_file) [HAVE_MKOSTEMP]: + Prefer mkostemp with O_CLOEXEC to mkstemp. + * callproc.c (relocate_fd) [!WINDOWSNT]: + * emacs.c (main): Use F_DUPFD_CLOEXEC, not plain F_DUPFD. + No need to use fcntl (..., F_SETFD, FD_CLOEXEC), since we're + now using pipe2. + * filelock.c (create_lock_file) [! HAVE_MKOSTEMP]: + Make the resulting file descriptor close-on-exec. + * lisp.h, lread.c, process.c (close_load_descs, close_process_descs): + * lread.c (load_descriptor_list, load_descriptor_unwind): + Remove; no longer needed. All uses removed. + * process.c (SOCK_CLOEXEC): Define to 0 if not supplied by system. + (close_on_exec, accept4, process_socket) [!SOCK_CLOEXEC]: + New functions. + (socket) [!SOCK_CLOEXEC]: Supply a substitute. + (Fmake_network_process, Fnetwork_interface_list): + (Fnetwork_interface_info, server_accept_connection): + Make newly-created socket close-on-exec. + * sysdep.c (emacs_open, emacs_fopen): + Make new-created descriptor close-on-exec. + * w32.c (fcntl): Support F_DUPFD_CLOEXEC well enough for Emacs. + * w32.c, w32.h (pipe2): Rename from 'pipe', with new flags arg. + 2013-07-07 Jan Djärv <jan.h.d@swipnet.se> * nsterm.m (sendEvent:): Propagate keyboard events to modal windows diff --git a/src/alloc.c b/src/alloc.c index a31a176caa7..b71cdb98d78 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -4741,7 +4741,7 @@ valid_pointer_p (void *p) Unfortunately, we cannot use NULL_DEVICE here, as emacs_write may not validate p in that case. */ - if (pipe (fd) == 0) + if (pipe2 (fd, O_CLOEXEC) == 0) { bool valid = emacs_write (fd[1], (char *) p, 16) == 16; emacs_close (fd[1]); diff --git a/src/callproc.c b/src/callproc.c index 185dc9a493e..3e70b1c2e49 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -519,7 +519,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS) * { #ifndef MSDOS int fd[2]; - if (pipe (fd) == -1) + if (pipe2 (fd, O_CLOEXEC) != 0) { int pipe_errno = errno; emacs_close (filefd); @@ -1036,12 +1036,16 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r memcpy (tempfile, SDATA (encoded_tem), SBYTES (encoded_tem) + 1); coding_systems = Qt; -#ifdef HAVE_MKSTEMP +#if defined HAVE_MKOSTEMP || defined HAVE_MKSTEMP { int fd; block_input (); +# ifdef HAVE_MKOSTEMP + fd = mkostemp (tempfile, O_CLOEXEC); +# else fd = mkstemp (tempfile); +# endif unblock_input (); if (fd == -1) report_file_error ("Failed to open temporary file", @@ -1184,15 +1188,6 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp, pid_t pid = getpid (); - /* Close Emacs's descriptors that this process should not have. */ - close_process_descs (); - - /* DOS_NT isn't in a vfork, so if we are in the middle of load-file, - we will lose if we call close_load_descs here. */ -#ifndef DOS_NT - close_load_descs (); -#endif - /* Note that use of alloca is always safe here. It's obvious for systems that do not have true vfork or that have true (stack) alloca. If using vfork and C_ALLOCA (when Emacs used to include @@ -1354,9 +1349,11 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp, emacs_close (1); emacs_close (2); + /* Redirect file descriptors and clear FD_CLOEXEC on the redirected ones. */ dup2 (in, 0); dup2 (out, 1); dup2 (err, 2); + emacs_close (in); if (out != in) emacs_close (out); @@ -1394,7 +1391,7 @@ relocate_fd (int fd, int minfd) return fd; else { - int new = fcntl (fd, F_DUPFD, minfd); + int new = fcntl (fd, F_DUPFD_CLOEXEC, minfd); if (new == -1) { const char *message_1 = "Error while setting up child: "; diff --git a/src/emacs.c b/src/emacs.c index 6ba01d1a443..e4412e2ea1a 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -889,7 +889,7 @@ main (int argc, char **argv) emacs_close (0); emacs_close (1); result = emacs_open (term, O_RDWR, 0); - if (result < 0 || dup (0) < 0) + if (result < 0 || fcntl (0, F_DUPFD_CLOEXEC, 1) < 0) { char *errstring = strerror (errno); fprintf (stderr, "%s: %s: %s\n", argv[0], term, errstring); @@ -969,7 +969,7 @@ main (int argc, char **argv) use a pipe for synchronization. The parent waits for the child to close its end of the pipe (using `daemon-initialized') before exiting. */ - if (pipe (daemon_pipe) == -1) + if (pipe2 (daemon_pipe, O_CLOEXEC) != 0) { fprintf (stderr, "Cannot pipe!\n"); exit (1); @@ -1065,9 +1065,6 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem daemon_name = xstrdup (dname_arg); /* Close unused reading end of the pipe. */ close (daemon_pipe[0]); - /* Make sure that the used end of the pipe is closed on exec, so - that it is not accessible to programs started from .emacs. */ - fcntl (daemon_pipe[1], F_SETFD, FD_CLOEXEC); setsid (); #else /* DOS_NT */ diff --git a/src/filelock.c b/src/filelock.c index de6aba8385c..1fcd2432484 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -416,8 +416,13 @@ create_lock_file (char *lfname, char *lock_info_str, bool force) memcpy (nonce, lfname, lfdirlen); strcpy (nonce + lfdirlen, nonce_base); -#if HAVE_MKSTEMP - /* Prefer mkstemp if available, as it avoids a race between +#if HAVE_MKOSTEMP + /* Prefer mkostemp to mkstemp, as it avoids a window where FD is + temporarily open without close-on-exec. */ + fd = mkostemp (nonce, O_BINARY | O_CLOEXEC); + need_fchmod = 1; +#elif HAVE_MKSTEMP + /* Prefer mkstemp to mktemp, as it avoids a race between mktemp and emacs_open. */ fd = mkstemp (nonce); need_fchmod = 1; @@ -432,7 +437,11 @@ create_lock_file (char *lfname, char *lock_info_str, bool force) err = errno; else { - ptrdiff_t lock_info_len = strlen (lock_info_str); + ptrdiff_t lock_info_len; +#if ! HAVE_MKOSTEMP + 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 || (need_fchmod && fchmod (fd, world_readable) != 0)) diff --git a/src/lisp.h b/src/lisp.h index 5d6fa760108..692f73cdbc3 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3668,7 +3668,6 @@ extern Lisp_Object string_to_number (char const *, int, bool); extern void map_obarray (Lisp_Object, void (*) (Lisp_Object, Lisp_Object), Lisp_Object); extern void dir_warning (const char *, Lisp_Object); -extern void close_load_descs (void); extern void init_obarray (void); extern void init_lread (void); extern void syms_of_lread (void); @@ -3995,7 +3994,6 @@ extern void delete_keyboard_wait_descriptor (int); extern void add_gpm_wait_descriptor (int); extern void delete_gpm_wait_descriptor (int); #endif -extern void close_process_descs (void); extern void init_process_emacs (void); extern void syms_of_process (void); extern void setup_process_coding_systems (Lisp_Object); diff --git a/src/lread.c b/src/lread.c index 2ceeb106653..83d2e8d954a 100644 --- a/src/lread.c +++ b/src/lread.c @@ -95,9 +95,6 @@ static Lisp_Object Qload_in_progress; It must be set to nil before all top-level calls to read0. */ static Lisp_Object read_objects; -/* List of descriptors now open for Fload. */ -static Lisp_Object load_descriptor_list; - /* File for get_file_char to read from. Use by load. */ static FILE *instream; @@ -149,7 +146,6 @@ static void readevalloop (Lisp_Object, FILE *, Lisp_Object, bool, Lisp_Object, Lisp_Object, Lisp_Object, Lisp_Object); static Lisp_Object load_unwind (Lisp_Object); -static Lisp_Object load_descriptor_unwind (Lisp_Object); /* Functions that read one byte from the current source READCHARFUN or unreads one byte. If the integer argument C is -1, it returns @@ -1328,11 +1324,8 @@ Return t if the file exists and loads successfully. */) } record_unwind_protect (load_unwind, make_save_pointer (stream)); - record_unwind_protect (load_descriptor_unwind, load_descriptor_list); specbind (Qload_file_name, found); specbind (Qinhibit_file_name_operation, Qnil); - load_descriptor_list - = Fcons (make_number (fileno (stream)), load_descriptor_list); specbind (Qload_in_progress, Qt); instream = stream; @@ -1395,26 +1388,6 @@ load_unwind (Lisp_Object arg) /* Used as unwind-protect function in load. */ } return Qnil; } - -static Lisp_Object -load_descriptor_unwind (Lisp_Object oldlist) -{ - load_descriptor_list = oldlist; - return Qnil; -} - -/* Close all descriptors in use for Floads. - This is used when starting a subprocess. */ - -void -close_load_descs (void) -{ -#ifndef WINDOWSNT - Lisp_Object tail; - for (tail = load_descriptor_list; CONSP (tail); tail = XCDR (tail)) - emacs_close (XFASTINT (XCAR (tail))); -#endif -} static bool complete_filename_p (Lisp_Object pathname) @@ -4376,9 +4349,6 @@ init_lread (void) load_in_progress = 0; Vload_file_name = Qnil; - - load_descriptor_list = Qnil; - Vstandard_input = Qt; Vloads_in_progress = Qnil; } @@ -4651,9 +4621,6 @@ variables, this must be set in the first line of a file. */); /* Vsource_directory was initialized in init_lread. */ - load_descriptor_list = Qnil; - staticpro (&load_descriptor_list); - DEFSYM (Qcurrent_load_list, "current-load-list"); DEFSYM (Qstandard_input, "standard-input"); DEFSYM (Qread_char, "read-char"); diff --git a/src/nsterm.m b/src/nsterm.m index dfb82edd738..074a5adf78c 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -4142,7 +4142,7 @@ ns_term_init (Lisp_Object display_name) if (selfds[0] == -1) { - if (pipe (selfds) == -1) + if (pipe2 (selfds, O_CLOEXEC) != 0) { fprintf (stderr, "Failed to create pipe: %s\n", emacs_strerror (errno)); diff --git a/src/process.c b/src/process.c index b77fb97168a..cad42470bc1 100644 --- a/src/process.c +++ b/src/process.c @@ -135,6 +135,34 @@ extern int sys_select (int, SELECT_TYPE *, SELECT_TYPE *, SELECT_TYPE *, EMACS_TIME *, void *); #endif +#ifndef SOCK_CLOEXEC +# define SOCK_CLOEXEC 0 + +/* Emulate GNU/Linux accept4 and socket well enough for this module. */ + +static int +close_on_exec (int fd) +{ + if (0 <= fd) + fcntl (fd, F_SETFD, FD_CLOEXEC); + return fd; +} + +static int +accept4 (int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags) +{ + return close_on_exec (accept (sockfd, addr, addrlen)); +} + +static int +process_socket (int domain, int type, int protocol) +{ + return close_on_exec (socket (domain, type, protocol)); +} +# undef socket +# define socket(domain, type, protocol) process_socket (domain, type, protocol) +#endif + /* Work around GCC 4.7.0 bug with strict overflow checking; see <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52904>. These lines can be removed once the GCC bug is fixed. */ @@ -1619,14 +1647,11 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) else #endif /* HAVE_PTYS */ { - int tem; - tem = pipe (sv); - if (tem < 0) + if (pipe2 (sv, O_CLOEXEC) != 0) report_file_error ("Creating pipe", Qnil); inchannel = sv[0]; forkout = sv[1]; - tem = pipe (sv); - if (tem < 0) + if (pipe2 (sv, O_CLOEXEC) != 0) { emacs_close (inchannel); emacs_close (forkout); @@ -1637,29 +1662,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) } #ifndef WINDOWSNT - { - int tem; - - tem = pipe (wait_child_setup); - if (tem < 0) - report_file_error ("Creating pipe", Qnil); - tem = fcntl (wait_child_setup[1], F_GETFD, 0); - if (tem >= 0) - tem = fcntl (wait_child_setup[1], F_SETFD, tem | FD_CLOEXEC); - if (tem < 0) - { - emacs_close (wait_child_setup[0]); - emacs_close (wait_child_setup[1]); - report_file_error ("Setting file descriptor flags", Qnil); - } - } + if (pipe2 (wait_child_setup, O_CLOEXEC) != 0) + report_file_error ("Creating pipe", Qnil); #endif fcntl (inchannel, F_SETFL, O_NONBLOCK); fcntl (outchannel, F_SETFL, O_NONBLOCK); - /* Record this as an active process, with its channels. - As a result, child_setup will close Emacs's side of the pipes. */ + /* Record this as an active process, with its channels. */ chan_process[inchannel] = process; XPROCESS (process)->infd = inchannel; XPROCESS (process)->outfd = outchannel; @@ -3135,7 +3145,8 @@ usage: (make-network-process &rest ARGS) */) retry_connect: #endif - s = socket (lres->ai_family, lres->ai_socktype, lres->ai_protocol); + s = socket (lres->ai_family, lres->ai_socktype | SOCK_CLOEXEC, + lres->ai_protocol); if (s < 0) { xerrno = errno; @@ -3532,7 +3543,7 @@ format; see the description of ADDRESS in `make-network-process'. */) int s; Lisp_Object res; - s = socket (AF_INET, SOCK_STREAM, 0); + s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0); if (s < 0) return Qnil; @@ -3688,7 +3699,7 @@ FLAGS is the current flags of the interface. */) error ("interface name too long"); strcpy (rq.ifr_name, SSDATA (ifname)); - s = socket (AF_INET, SOCK_STREAM, 0); + s = socket (AF_INET, SOCK_STREAM | SOCK_CLOEXEC, 0); if (s < 0) return Qnil; @@ -3984,7 +3995,7 @@ server_accept_connection (Lisp_Object server, int channel) } saddr; socklen_t len = sizeof saddr; - s = accept (channel, &saddr.sa, &len); + s = accept4 (channel, &saddr.sa, &len, SOCK_CLOEXEC); if (s < 0) { @@ -6858,32 +6869,6 @@ setup_process_coding_systems (Lisp_Object process) #endif } -/* Close all descriptors currently in use for communication - with subprocess. This is used in a newly-forked subprocess - to get rid of irrelevant descriptors. */ - -void -close_process_descs (void) -{ -#ifndef DOS_NT - int i; - for (i = 0; i < MAXDESC; i++) - { - Lisp_Object process; - process = chan_process[i]; - if (!NILP (process)) - { - int in = XPROCESS (process)->infd; - int out = XPROCESS (process)->outfd; - if (in >= 0) - emacs_close (in); - if (out >= 0 && in != out) - emacs_close (out); - } - } -#endif -} - DEFUN ("get-buffer-process", Fget_buffer_process, Sget_buffer_process, 1, 1, 0, doc: /* Return the (or a) process associated with BUFFER. BUFFER may be a buffer or the name of one. */) diff --git a/src/sysdep.c b/src/sysdep.c index 91e941a2050..faca7fae461 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -543,8 +543,6 @@ sys_subshell (void) #endif } - close_process_descs (); /* Close Emacs's pipes/ptys */ - #ifdef MSDOS /* Demacs 1.1.2 91/10/20 Manabu Higashida */ { char *epwd = getenv ("PWD"); @@ -2152,6 +2150,8 @@ emacs_abort (void) #endif /* Open FILE for Emacs use, using open flags OFLAG and mode MODE. + Arrange for subprograms to not inherit the file descriptor. + Prefer a method that is multithread-safe, if available. Do not fail merely because the open was interrupted by a signal. Allow the user to quit. */ @@ -2159,8 +2159,11 @@ int emacs_open (const char *file, int oflags, int mode) { int fd; + oflags |= O_CLOEXEC; while ((fd = open (file, oflags, mode)) < 0 && errno == EINTR) QUIT; + if (! O_CLOEXEC && 0 <= fd) + fcntl (fd, F_SETFD, FD_CLOEXEC); return fd; } @@ -2170,10 +2173,29 @@ emacs_open (const char *file, int oflags, int mode) FILE * emacs_fopen (char const *file, char const *mode) { - FILE *fp; - while (! (fp = fopen (file, mode)) && errno == EINTR) - QUIT; - return fp; + int fd, omode, oflags; + int bflag = 0; + char const *m = mode; + + switch (*m++) + { + case 'r': omode = O_RDONLY; oflags = 0; break; + case 'w': omode = O_WRONLY; oflags = O_CREAT | O_TRUNC; break; + case 'a': omode = O_WRONLY; oflags = O_CREAT | O_APPEND; break; + default: emacs_abort (); + } + + while (*m) + switch (*m++) + { + case '+': omode = O_RDWR; break; + case 'b': bflag = O_BINARY; break; + case 't': bflag = O_TEXT; break; + default: /* Ignore. */ break; + } + + fd = emacs_open (file, omode | oflags | bflag, 0666); + return fd < 0 ? 0 : fdopen (fd, mode); } int diff --git a/src/w32.c b/src/w32.c index 230479cd61a..46aebe8b634 100644 --- a/src/w32.c +++ b/src/w32.c @@ -6719,10 +6719,16 @@ sys_sendto (int s, const char * buf, int len, int flags, } /* Windows does not have an fcntl function. Provide an implementation - solely for making sockets non-blocking. */ + good enough for Emacs. */ int fcntl (int s, int cmd, int options) { + /* In the w32 Emacs port, fcntl (fd, F_DUPFD_CLOEXEC, fd1) is always + invoked in a context where fd1 is closed and all descriptors less + than fd1 are open, so sys_dup is an adequate implementation. */ + if (cmd == F_DUPFD_CLOEXEC) + return sys_dup (s); + if (winsock_lib == NULL) { errno = ENETDOWN; @@ -6864,13 +6870,14 @@ sys_dup2 (int src, int dst) return rc; } -/* Unix pipe() has only one arg */ int -sys_pipe (int * phandles) +pipe2 (int * phandles, int pipe2_flags) { int rc; unsigned flags; + eassert (pipe2_flags == O_CLOEXEC); + /* make pipe handles non-inheritable; when we spawn a child, we replace the relevant handle with an inheritable one. Also put pipes into binary mode; we will do text mode translation ourselves diff --git a/src/w32.h b/src/w32.h index 17da0778db1..9c1f1efa699 100644 --- a/src/w32.h +++ b/src/w32.h @@ -188,7 +188,7 @@ extern int random (void); extern int fchmod (int, mode_t); extern int sys_rename_replace (char const *, char const *, BOOL); -extern int sys_pipe (int *); +extern int pipe2 (int *, int); extern void set_process_dir (char *); extern int sys_spawnve (int, char *, char **, char **); |