From 6496aec9e948e0e5e6646aff408391d78f3516cc Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 20 Jul 2013 08:33:00 -0700 Subject: [PATCH] Fix array bounds violation when pty allocation fails. * configure.ac (PTY_TTY_NAME_SPRINTF): Use PTY_NAME_SIZE, not sizeof pty_name, since pty_name is now a pointer to the array. * src/process.c (PTY_NAME_SIZE): New constant. (pty_name): Remove static variable; it's now auto. (allocate_pty): Define even if !HAVE_PTYS; that's simpler. Take pty_name as an arg rather than using a static variable. All callers changed. (create_process): Recover pty_flag from process, not from volatile local. (create_pty): Stay inside array even when pty allocation fails. (Fmake_serial_process): Omit unnecessary initializaiton of pty_flag. --- ChangeLog | 6 ++++ configure.ac | 6 ++-- src/ChangeLog | 10 ++++++ src/process.c | 87 ++++++++++++++++++++++----------------------------- 4 files changed, 57 insertions(+), 52 deletions(-) diff --git a/ChangeLog b/ChangeLog index 967284b485d..ec76b4257e4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2013-07-20 Paul Eggert + + Fix array bounds violation when pty allocation fails. + * configure.ac (PTY_TTY_NAME_SPRINTF): Use PTY_NAME_SIZE, + not sizeof pty_name, since pty_name is now a pointer to the array. + 2013-07-13 Paul Eggert * configure.ac: Simplify --with-file-notification handling. diff --git a/configure.ac b/configure.ac index 0fdb1699e99..dfe2181fce8 100644 --- a/configure.ac +++ b/configure.ac @@ -3938,7 +3938,7 @@ case $opsys in AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)]) dnl Note that grantpt and unlockpt may fork. We must block SIGCHLD dnl to prevent sigchld_handler from intercepting the child's death. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname = 0; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); if (grantpt (fd) != -1 && unlockpt (fd) != -1) ptyname = ptsname(fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (!ptyname) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname = 0; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); if (grantpt (fd) != -1 && unlockpt (fd) != -1) ptyname = ptsname(fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (!ptyname) { emacs_close (fd); return -1; } snprintf (pty_name, PTY_NAME_SIZE, "%s", ptyname); }]) dnl if HAVE_POSIX_OPENPT if test "x$ac_cv_func_posix_openpt" = xyes; then AC_DEFINE(PTY_OPEN, [fd = posix_openpt (O_RDWR | O_CLOEXEC | O_NOCTTY)]) @@ -3986,12 +3986,12 @@ case $opsys in dnl On SysVr4, grantpt(3) forks a subprocess, so keep sigchld_handler() dnl from intercepting that death. If any child but grantpt's should die dnl within, it should be caught after sigrelse(2). - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); grantpt_result = grantpt (fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (grantpt_result == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); grantpt_result = grantpt (fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (grantpt_result == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, PTY_NAME_SIZE, "%s", ptyname); }]) ;; unixware ) dnl Comments are as per sol2*. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); grantpt_result = grantpt (fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (grantpt_result == -1) fatal("could not grant slave pty"); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, 0); grantpt_result = grantpt (fd); pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (grantpt_result == -1) fatal("could not grant slave pty"); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, PTY_NAME_SIZE, "%s", ptyname); }]) ;; esac diff --git a/src/ChangeLog b/src/ChangeLog index c88951f9422..4c6bfaa2b1a 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,15 @@ 2013-07-20 Paul Eggert + Fix array bounds violation when pty allocation fails. + * process.c (PTY_NAME_SIZE): New constant. + (pty_name): Remove static variable; it's now auto. + (allocate_pty): Define even if !HAVE_PTYS; that's simpler. + Take pty_name as an arg rather than using a static variable. + All callers changed. + (create_process): Recover pty_flag from process, not from volatile local. + (create_pty): Stay inside array even when pty allocation fails. + (Fmake_serial_process): Omit unnecessary initializaiton of pty_flag. + * lread.c (Fload): Avoid initialization only when lint checking. Mention that it's needed only for older GCCs. diff --git a/src/process.c b/src/process.c index f4ae662468b..12035da7b58 100644 --- a/src/process.c +++ b/src/process.c @@ -640,19 +640,16 @@ status_message (struct Lisp_Process *p) return Fcopy_sequence (Fsymbol_name (symbol)); } -#ifdef HAVE_PTYS - -/* The file name of the pty opened by allocate_pty. */ -static char pty_name[24]; +enum { PTY_NAME_SIZE = 24 }; /* Open an available pty, returning a file descriptor. - Return -1 on failure. - The file name of the terminal corresponding to the pty - is left in the variable pty_name. */ + Store into PTY_NAME the file name of the terminal corresponding to the pty. + Return -1 on failure. */ static int -allocate_pty (void) +allocate_pty (char pty_name[PTY_NAME_SIZE]) { +#ifdef HAVE_PTYS int fd; #ifdef PTY_ITERATION @@ -697,9 +694,9 @@ allocate_pty (void) return fd; } } +#endif /* HAVE_PTYS */ return -1; } -#endif /* HAVE_PTYS */ static Lisp_Object make_process (Lisp_Object name) @@ -1621,14 +1618,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) #endif int forkin, forkout; bool pty_flag = 0; + char pty_name[PTY_NAME_SIZE]; Lisp_Object lisp_pty_name = Qnil; Lisp_Object encoded_current_dir; inchannel = outchannel = -1; -#ifdef HAVE_PTYS if (!NILP (Vprocess_connection_type)) - outchannel = inchannel = allocate_pty (); + outchannel = inchannel = allocate_pty (pty_name); if (inchannel >= 0) { @@ -1647,7 +1644,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) lisp_pty_name = build_string (pty_name); } else -#endif /* HAVE_PTYS */ { if (emacs_pipe (sv) != 0) report_file_error ("Creating pipe", Qnil); @@ -1704,7 +1700,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) Lisp_Object volatile encoded_current_dir_volatile = encoded_current_dir; Lisp_Object volatile lisp_pty_name_volatile = lisp_pty_name; Lisp_Object volatile process_volatile = process; - bool volatile pty_flag_volatile = pty_flag; char **volatile new_argv_volatile = new_argv; int volatile forkin_volatile = forkin; int volatile forkout_volatile = forkout; @@ -1716,12 +1711,13 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) encoded_current_dir = encoded_current_dir_volatile; lisp_pty_name = lisp_pty_name_volatile; process = process_volatile; - pty_flag = pty_flag_volatile; new_argv = new_argv_volatile; forkin = forkin_volatile; forkout = forkout_volatile; wait_child_setup[0] = wait_child_setup_0_volatile; wait_child_setup[1] = wait_child_setup_1_volatile; + + pty_flag = XPROCESS (process)->pty_flag; } if (pid == 0) @@ -1791,15 +1787,15 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) if (pty_flag) { - /* I wonder if emacs_close (emacs_open (pty_name, ...)) + /* I wonder if emacs_close (emacs_open (SSDATA (lisp_pty_name), ...)) would work? */ if (xforkin >= 0) emacs_close (xforkin); - xforkout = xforkin = emacs_open (pty_name, O_RDWR, 0); + xforkout = xforkin = emacs_open (SSDATA (lisp_pty_name), O_RDWR, 0); if (xforkin < 0) { - emacs_perror (pty_name); + emacs_perror (SSDATA (lisp_pty_name)); _exit (EXIT_CANCELED); } @@ -1899,17 +1895,16 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) } } -void +static void create_pty (Lisp_Object process) { + char pty_name[PTY_NAME_SIZE]; int inchannel, outchannel; - bool pty_flag = 0; inchannel = outchannel = -1; -#ifdef HAVE_PTYS if (!NILP (Vprocess_connection_type)) - outchannel = inchannel = allocate_pty (); + outchannel = inchannel = allocate_pty (pty_name); if (inchannel >= 0) { @@ -1928,40 +1923,34 @@ create_pty (Lisp_Object process) child_setup_tty (forkout); #endif /* DONT_REOPEN_PTY */ #endif /* not USG, or USG_SUBTTY_WORKS */ - pty_flag = 1; - } -#endif /* HAVE_PTYS */ - fcntl (inchannel, F_SETFL, O_NONBLOCK); - fcntl (outchannel, F_SETFL, O_NONBLOCK); + 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. */ - chan_process[inchannel] = process; - XPROCESS (process)->infd = inchannel; - XPROCESS (process)->outfd = outchannel; + /* Record this as an active process, with its channels. + As a result, child_setup will close Emacs's side of the pipes. */ + chan_process[inchannel] = process; + XPROCESS (process)->infd = inchannel; + XPROCESS (process)->outfd = outchannel; - /* Previously we recorded the tty descriptor used in the subprocess. - It was only used for getting the foreground tty process, so now - we just reopen the device (see emacs_get_tty_pgrp) as this is - more portable (see USG_SUBTTY_WORKS above). */ + /* Previously we recorded the tty descriptor used in the subprocess. + It was only used for getting the foreground tty process, so now + we just reopen the device (see emacs_get_tty_pgrp) as this is + more portable (see USG_SUBTTY_WORKS above). */ - XPROCESS (process)->pty_flag = pty_flag; - pset_status (XPROCESS (process), Qrun); - setup_process_coding_systems (process); + XPROCESS (process)->pty_flag = 1; + pset_status (XPROCESS (process), Qrun); + setup_process_coding_systems (process); - FD_SET (inchannel, &input_wait_mask); - FD_SET (inchannel, &non_keyboard_wait_mask); - if (inchannel > max_process_desc) - max_process_desc = inchannel; + FD_SET (inchannel, &input_wait_mask); + FD_SET (inchannel, &non_keyboard_wait_mask); + if (inchannel > max_process_desc) + max_process_desc = inchannel; + + pset_tty_name (XPROCESS (process), build_string (pty_name)); + } XPROCESS (process)->pid = -2; -#ifdef HAVE_PTYS - if (pty_flag) - pset_tty_name (XPROCESS (process), build_string (pty_name)); - else -#endif - pset_tty_name (XPROCESS (process), Qnil); } @@ -2589,7 +2578,7 @@ usage: (make-serial-process &rest ARGS) */) p->kill_without_query = 1; if (tem = Fplist_get (contact, QCstop), !NILP (tem)) pset_command (p, Qt); - p->pty_flag = 0; + eassert (! p->pty_flag); if (!EQ (p->command, Qt)) { -- 2.39.2