isa/sudo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Run the editor in its own process group.

Browse Source 
This fixes suspending the editor on GNU Hurd which doesn't seem to
have proper process group signal handling.
This commit is contained in:
Todd C. Miller
2023-02-21 16:14:14 -07:00
parent 1bcddb9602
commit 0339337103
13 changed files with 110 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
MANIFEST
View File

@@ -360,6 +360,7 @@ lib/util/strtonum.c
lib/util/sudo_conf.c
lib/util/sudo_debug.c
lib/util/sudo_dso.c
lib/util/suspend_parent.c
lib/util/sys_siglist.h
lib/util/sys_signame.h
lib/util/term.c
@@ -599,6 +600,7 @@ plugins/sudoers/b64_encode.c
plugins/sudoers/boottime.c
plugins/sudoers/bsm_audit.c
plugins/sudoers/bsm_audit.h
plugins/sudoers/canon_path.c
plugins/sudoers/check.c
plugins/sudoers/check.h
plugins/sudoers/check_aliases.c
@@ -740,7 +742,6 @@ plugins/sudoers/prompt.c
plugins/sudoers/pwutil.c
plugins/sudoers/pwutil.h
plugins/sudoers/pwutil_impl.c
plugins/sudoers/canon_path.c
plugins/sudoers/redblack.c
plugins/sudoers/redblack.h
plugins/sudoers/regress/check_symbols/check_symbols.c
@@ -1269,8 +1270,6 @@ src/sudo_intercept_common.c
src/sudo_noexec.c
src/sudo_plugin_int.h
src/sudo_usage.h.in
src/suspend_nopty.c
src/tcsetpgrp_nobg.c
src/tgetpass.c
src/ttyname.c
src/utmp.c

4
include/sudo_util.h
View File

@@ -344,6 +344,10 @@ sudo_dso_public int sudo_strtomode_v1(const char *cp, const char **errstr);
/* sudo_printf.c */
extern int (*sudo_printf)(int msg_type, const char *fmt, ...);
/* suspend_parent.c */
sudo_dso_public void sudo_suspend_parent_v1(int signo, pid_t my_pid, pid_t my_pgrp, pid_t cmnd_pid, void *closure, void (*callback)(void *, int));
#define sudo_suspend_parent(_a, _b, _c, _d, _e, _f) sudo_suspend_parent_v1((_a), (_b), (_c), (_d), (_e), (_f))
/* term.c */
sudo_dso_public bool sudo_term_cbreak_v1(int fd);
#define sudo_term_cbreak(_a) sudo_term_cbreak_v1((_a))

8
lib/util/Makefile.in
View File

@@ -150,7 +150,7 @@ LTOBJS = basename.lo @DIGEST@ event.lo fatal.lo key_val.lo gethostname.lo \
multiarch.lo parseln.lo progname.lo rcstr.lo regex.lo roundup.lo \
secure_path.lo setgroups.lo strsplit.lo strtobool.lo strtoid.lo \
strtomode.lo strtonum.lo sudo_conf.lo sudo_debug.lo sudo_dso.lo \
term.lo ttyname_dev.lo ttysize.lo uuid.lo \
suspend_parent.lo term.lo ttyname_dev.lo ttysize.lo uuid.lo \
@COMMON_OBJS@ @LTLIBOBJS@
IOBJS = $(LTOBJS:.lo=.i)
@@ -1629,6 +1629,12 @@ sudo_dso.i: $(srcdir)/sudo_dso.c $(incdir)/compat/stdbool.h \
$(CC) -E -o $@ $(CPPFLAGS) $<
sudo_dso.plog: sudo_dso.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/sudo_dso.c --i-file $< --output-file $@
suspend_parent.lo: $(srcdir)/suspend_parent.c $(top_builddir)/config.h
$(LIBTOOL) $(LTFLAGS) --mode=compile $(CC) -c -o $@ $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/suspend_parent.c
suspend_parent.i: $(srcdir)/suspend_parent.c $(top_builddir)/config.h
$(CC) -E -o $@ $(CPPFLAGS) $<
suspend_parent.plog: suspend_parent.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/suspend_parent.c --i-file $< --output-file $@
term.lo: $(srcdir)/term.c $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_queue.h $(incdir)/sudo_util.h \
$(top_builddir)/config.h

75
src/suspend_nopty.c → lib/util/suspend_parent.c
View File

@@ -1,7 +1,7 @@
/*
* SPDX-License-Identifier: ISC
*
* Copyright (c) 2009-2022 Todd C. Miller <Todd.Miller@sudo.ws>
* Copyright (c) 2009-2023 Todd C. Miller <Todd.Miller@sudo.ws>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
@@ -30,17 +30,66 @@
#include <fcntl.h>
#include <signal.h>
#include "sudo.h"
#include "sudo_exec.h"
#include "pathnames.h"
#include "sudo_debug.h"
#include "sudo_fatal.h"
#include "sudo_gettext.h"
#include "sudo_util.h"
static volatile sig_atomic_t got_sigttou;
/*
* SIGTTOU signal handler for term_restore that just sets a flag.
*/
static void
sigttou(int signo)
{
got_sigttou = 1;
}
/*
* Like tcsetpgrp() but restarts on EINTR _except_ for SIGTTOU.
* Returns 0 on success or -1 on failure, setting errno.
* Sets got_sigttou on failure if interrupted by SIGTTOU.
*/
static int
tcsetpgrp_nobg(int fd, pid_t pgrp_id)
{
struct sigaction sa, osa;
int rc;
debug_decl(tcsetpgrp_nobg, SUDO_DEBUG_UTIL);
/*
* If we receive SIGTTOU from tcsetpgrp() it means we are
* not in the foreground process group.
* This avoid a TOCTOU race compared to using tcgetpgrp().
*/
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0; /* do not restart syscalls */
sa.sa_handler = sigttou;
got_sigttou = 0;
(void)sigaction(SIGTTOU, &sa, &osa);
do {
rc = tcsetpgrp(fd, pgrp_id);
} while (rc != 0 && errno == EINTR && !got_sigttou);
(void)sigaction(SIGTTOU, &osa, NULL);
debug_return_int(rc);
}
/*
* Suspend the main process in response to an interactive child process
* being suspended.
*/
void
suspend_sudo_nopty(struct exec_closure *ec, int signo, pid_t my_pid,
pid_t my_pgrp, pid_t cmnd_pid)
sudo_suspend_parent_v1(int signo, pid_t my_pid, pid_t my_pgrp, pid_t cmnd_pid,
void *closure, void (*callback)(void *, int))
{
struct sigaction sa, osa;
pid_t saved_pgrp = -1;
int fd;
debug_decl(suspend_sudo_nopty, SUDO_DEBUG_EXEC);
debug_decl(sudo_suspend_parent, SUDO_DEBUG_EXEC);
/*
* Save the controlling terminal's process group so we can restore
@@ -81,26 +130,28 @@ suspend_sudo_nopty(struct exec_closure *ec, int signo, pid_t my_pid,
}
}
/* Log the suspend event. */
log_suspend(ec, signo);
/* Run callback before we suspend. */
if (callback != NULL)
callback(callback, signo);
if (signo == SIGTSTP) {
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESTART;
sa.sa_handler = SIG_DFL;
if (sudo_sigaction(SIGTSTP, &sa, &osa) != 0)
if (sigaction(SIGTSTP, &sa, &osa) != 0)
sudo_warn(U_("unable to set handler for signal %d"), SIGTSTP);
}
if (kill(my_pid, signo) != 0)
sudo_warn("kill(%d, %d)", (int)my_pid, signo);
if (signo == SIGTSTP) {
if (sudo_sigaction(SIGTSTP, &osa, NULL) != 0)
if (sigaction(SIGTSTP, &osa, NULL) != 0)
sudo_warn(U_("unable to restore handler for signal %d"), SIGTSTP);
}
/* Log the resume event. */
log_suspend(ec, SIGCONT);
/* Run callback on resume. */
if (callback != NULL)
callback(callback, SIGCONT);
if (saved_pgrp != -1) {
/*

1
lib/util/util.exp.in
View File

@@ -144,6 +144,7 @@ sudo_strtoid_v2
sudo_strtoidx_v1
sudo_strtomode_v1
sudo_strtonum
sudo_suspend_parent_v1
sudo_term_cbreak_v1
sudo_term_copy_v1
sudo_term_eof

32
plugins/sudoers/visudo.c
View File

@@ -93,7 +93,7 @@ static bool install_sudoers(struct sudoersfile *, bool, bool);
static bool visudo_track_error(const char *file, int line, int column, const char *fmt, va_list args) sudo_printf0like(4, 0);
static int print_unused(struct sudoers_parse_tree *, struct alias *, void *);
static bool reparse_sudoers(char *, int, char **, bool, bool);
static int run_command(const char *, char *const *);
static int run_command(const char *, char *const *, bool);
static void parse_sudoers_options(void);
static void setup_signals(void);
static void visudo_cleanup(void);
@@ -557,7 +557,7 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
goto done;
}
if (run_command(editor, editor_argv) != -1) {
if (run_command(editor, editor_argv, true) != -1) {
if (sudo_gettime_real(&times[1]) == -1) {
sudo_warn("%s", U_("unable to read the clock"));
goto done;
@@ -820,7 +820,7 @@ install_sudoers(struct sudoersfile *sp, bool set_owner, bool set_mode)
av[3] = NULL;
/* And run it... */
if (run_command(_PATH_MV, av) != 0) {
if (run_command(_PATH_MV, av, false) != 0) {
sudo_warnx(U_("command failed: '%s %s %s', %s unchanged"),
_PATH_MV, sp->tpath, sp->path, sp->path);
goto done;
@@ -895,9 +895,9 @@ setup_signals(void)
}
static int
run_command(const char *path, char *const *argv)
run_command(const char *path, char *const *argv, bool foreground)
{
int status;
int fd, status;
pid_t pid, rv;
debug_decl(run_command, SUDOERS_DEBUG_UTIL);
@@ -906,6 +906,14 @@ run_command(const char *path, char *const *argv)
sudo_fatal(U_("unable to execute %s"), path);
break; /* NOTREACHED */
case 0:
/* Run command as the foreground process of a new process group. */
if (foreground) {
pid = getpid();
setpgid(0, pid);
fd = open(_PATH_TTY, O_RDWR);
if (fd != -1)
tcsetpgrp(fd, pid);
}
closefrom(STDERR_FILENO + 1);
execv(path, argv);
sudo_warn(U_("unable to run %s"), path);
@@ -914,11 +922,19 @@ run_command(const char *path, char *const *argv)
}
for (;;) {
rv = waitpid(pid, &status, 0);
if (rv == -1 && errno != EINTR)
rv = waitpid(pid, &status, WUNTRACED);
if (rv == -1) {
if (errno == EINTR)
continue;
break;
if (rv != -1 && !WIFSTOPPED(status))
}
if (!WIFSTOPPED(status))
break;
if (foreground) {
sudo_suspend_parent(WSTOPSIG(status), getpid(), getpgrp(),
pid, NULL, NULL);
killpg(pid, SIGCONT);
}
}
if (rv != -1)

41
src/Makefile.in
View File

@@ -145,8 +145,7 @@ OBJS = conversation.o copy_file.o edit_open.o env_hooks.o exec.o \
exec_common.o exec_intercept.o exec_iolog.o exec_monitor.o \
exec_nopty.o exec_preload.o exec_ptrace.o exec_pty.o get_pty.o \
hooks.o limits.o load_plugins.o net_ifs.o parse_args.o preserve_fds.o \
signal.o sudo.o sudo_edit.o suspend_nopty.o tcsetpgrp_nobg.o \
tgetpass.o ttyname.o utmp.o @SUDO_OBJS@
signal.o sudo.o sudo_edit.o tgetpass.o ttyname.o utmp.o @SUDO_OBJS@
IOBJS = $(OBJS:.o=.i) sesh.i sudo_intercept.i sudo_intercept_common.i
@@ -163,7 +162,7 @@ CHECK_NOEXEC_OBJS = check_noexec.o exec_common.o exec_preload.o
CHECK_TTYNAME_OBJS = check_ttyname.o ttyname.o
TEST_PTRACE_OBJS = suspend_nopty.o tcsetpgrp_nobg.o test_ptrace.o
TEST_PTRACE_OBJS = test_ptrace.o
LIBOBJDIR = $(top_builddir)/@ac_config_libobj_dir@/
@@ -1005,42 +1004,6 @@ sudo_intercept_common.i: $(srcdir)/sudo_intercept_common.c \
$(CC) -E -o $@ $(CPPFLAGS) $<
sudo_intercept_common.plog: sudo_intercept_common.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/sudo_intercept_common.c --i-file $< --output-file $@
suspend_nopty.o: $(srcdir)/suspend_nopty.c $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
$(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
$(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
$(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \
$(top_builddir)/config.h $(top_builddir)/pathnames.h
$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/suspend_nopty.c
suspend_nopty.i: $(srcdir)/suspend_nopty.c $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
$(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
$(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
$(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \
$(top_builddir)/config.h $(top_builddir)/pathnames.h
$(CC) -E -o $@ $(CPPFLAGS) $<
suspend_nopty.plog: suspend_nopty.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/suspend_nopty.c --i-file $< --output-file $@
tcsetpgrp_nobg.o: $(srcdir)/tcsetpgrp_nobg.c $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
$(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
$(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
$(incdir)/sudo_util.h $(srcdir)/sudo.h \
$(top_builddir)/config.h $(top_builddir)/pathnames.h
$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(HARDENING_CFLAGS) $(srcdir)/tcsetpgrp_nobg.c
tcsetpgrp_nobg.i: $(srcdir)/tcsetpgrp_nobg.c $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
$(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
$(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
$(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
$(incdir)/sudo_util.h $(srcdir)/sudo.h \
$(top_builddir)/config.h $(top_builddir)/pathnames.h
$(CC) -E -o $@ $(CPPFLAGS) $<
tcsetpgrp_nobg.plog: tcsetpgrp_nobg.i
rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/tcsetpgrp_nobg.c --i-file $< --output-file $@
test_ptrace.o: $(srcdir)/regress/intercept/test_ptrace.c \
$(incdir)/compat/endian.h $(incdir)/compat/stdbool.h \
$(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \

3
src/exec_iolog.c
View File

@@ -533,8 +533,9 @@ log_stderr(const char *buf, unsigned int n, struct io_buffer *iob)
/* Call I/O plugin suspend log method. */
void
log_suspend(struct exec_closure *ec, int signo)
log_suspend(void *v, int signo)
{
struct exec_closure *ec = v;
struct plugin_container *plugin;
const char *errstr = NULL;
sigset_t omask;

4
src/exec_nopty.c
View File

@@ -758,8 +758,8 @@ handle_sigchld_nopty(struct exec_closure *ec)
/* If the main command is suspended, suspend sudo too. */
if (pid == ec->cmnd_pid) {
suspend_sudo_nopty(ec, signo, ec->sudo_pid, ec->ppgrp,
ec->cmnd_pid);
sudo_suspend_parent(signo, ec->sudo_pid, ec->ppgrp,
ec->cmnd_pid, ec, log_suspend);
}
} else {
if (WIFSIGNALED(status)) {

11
src/regress/intercept/test_ptrace.c
View File

@@ -104,13 +104,6 @@ sudo_sigaction(int signo, struct sigaction *sa, struct sigaction *osa)
return sigaction(signo, sa, osa);
}
/* STUB */
void
log_suspend(struct exec_closure *ec, int signo)
{
return;
}
int
main(int argc, char *argv[])
{
@@ -228,8 +221,8 @@ main(int argc, char *argv[])
} else if (WIFSTOPPED(status)) {
if (exec_ptrace_stopped(pid, status, &closure)) {
if (pid == child) {
suspend_sudo_nopty(NULL, WSTOPSIG(status), my_pid,
my_pgrp, child);
sudo_suspend_parent(WSTOPSIG(status), my_pid,
my_pgrp, child, NULL, NULL);
if (kill(child, SIGCONT) != 0)
sudo_warn("kill(%d, SIGCONT)", (int)child);
}

3
src/sudo.h
View File

@@ -336,9 +336,6 @@ int add_preserved_fd(struct preserved_fd_list *pfds, int fd);
void closefrom_except(int startfd, struct preserved_fd_list *pfds);
void parse_preserved_fds(struct preserved_fd_list *pfds, const char *fdstr);
/* setpgrp_nobg.c */
int tcsetpgrp_nobg(int fd, pid_t pgrp_id);
/* limits.c */
void disable_coredump(void);
void restore_limits(void);

5
src/sudo_exec.h
View File

@@ -194,7 +194,7 @@ bool log_stdin(const char *buf, unsigned int n, struct io_buffer *iob);
bool log_ttyout(const char *buf, unsigned int n, struct io_buffer *iob);
bool log_stdout(const char *buf, unsigned int n, struct io_buffer *iob);
bool log_stderr(const char *buf, unsigned int n, struct io_buffer *iob);
void log_suspend(struct exec_closure *ec, int signo);
void log_suspend(void *v, int signo);
void log_winchange(struct exec_closure *ec, unsigned int rows, unsigned int cols);
void io_buf_new(int rfd, int wfd, bool (*action)(const char *, unsigned int, struct io_buffer *), void (*read_cb)(int fd, int what, void *v), void (*write_cb)(int fd, int what, void *v), struct exec_closure *ec, struct io_buffer_list *head);
int safe_close(int fd);
@@ -232,7 +232,4 @@ bool exec_ptrace_stopped(pid_t pid, int status, void *intercept);
bool set_exec_filter(void);
int exec_ptrace_seize(pid_t child);
/* suspend_nopty.c */
void suspend_sudo_nopty(struct exec_closure *ec, int signo, pid_t my_pid, pid_t my_pgrp, pid_t cmnd_pid);
#endif /* SUDO_EXEC_H */

71
src/tcsetpgrp_nobg.c
View File

@@ -1,71 +0,0 @@
/*
* SPDX-License-Identifier: ISC
*
* Copyright (c) 2017 Todd C. Miller <Todd.Miller@sudo.ws>
*
* Permission to use, copy, modify, and distribute this software for any
* purpose with or without fee is hereby granted, provided that the above
* copyright notice and this permission notice appear in all copies.
*
* THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
* WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
* ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
* WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
* ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
* OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
/*
* This is an open source non-commercial project. Dear PVS-Studio, please check it.
* PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
*/
#include <config.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <signal.h>
#include "sudo.h"
static volatile sig_atomic_t got_sigttou;
/*
* SIGTTOU signal handler for tcsetpgrp_nobg() that just sets a flag.
*/
static void
sigttou(int signo)
{
got_sigttou = 1;
}
/*
* Like tcsetpgrp() but restarts on EINTR _except_ for SIGTTOU.
* Returns 0 on success or -1 on failure, setting errno.
* Sets got_sigttou on failure if interrupted by SIGTTOU.
*/
int
tcsetpgrp_nobg(int fd, pid_t pgrp_id)
{
struct sigaction sa, osa;
int rc;
/*
* If we receive SIGTTOU from tcsetpgrp() it means we are
* not in the foreground process group.
* This avoid a TOCTOU race compared to using tcgetpgrp().
*/
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = 0; /* do not restart syscalls */
sa.sa_handler = sigttou;
got_sigttou = 0;
(void)sigaction(SIGTTOU, &sa, &osa);
do {
rc = tcsetpgrp(fd, pgrp_id);
} while (rc != 0 && errno == EINTR && !got_sigttou);
(void)sigaction(SIGTTOU, &osa, NULL);
return rc;
}