summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Blake <ebb9@byu.net>2009-10-28 14:36:09 -0600
committerEric Blake <ebb9@byu.net>2009-10-28 21:12:41 -0600
commit1c59bb3cefff73c532033863e60e9130892a50dd (patch)
tree42f89ad649d5be625ac200f044c3f7acd5ee5e08
parent536a1fbe5ff47078d515a41ea4b45c4e0d794da2 (diff)
downloadcoreutils-1c59bb3cefff73c532033863e60e9130892a50dd.tar.xz
nice, nohup, su: detect write failure to stderr
These programs can print non-fatal diagnostics to stderr prior to exec'ing a subsidiary program. However, if we thought the situation warranted a diagnostic, we insist that the diagnostic be printed without error, rather than blindly exec, as it may be a security risk. For an example, try 'nice -n -1 nice 2>/dev/full'. Failure to raise priority (by lowering niceness) is not fatal, but failure to inform the user about failure to change priority is dangerous. * src/nice.c (main): Declare failure if writing advisory message to stderr fails. * src/nohup.c (main): Likewise. * src/su.c (main): Likewise. * tests/misc/nice: Test this. * tests/misc/nohup: Likewise. * NEWS: Document this.
-rw-r--r--NEWS4
-rw-r--r--src/nice.c13
-rw-r--r--src/nohup.c9
-rw-r--r--src/su.c8
-rwxr-xr-xtests/misc/nice6
-rwxr-xr-xtests/misc/nohup15
6 files changed, 53 insertions, 2 deletions
diff --git a/NEWS b/NEWS
index abf2466b6..076077525 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ GNU coreutils NEWS -*- outline -*-
call fails with errno == EACCES.
[the bug dates back to the initial implementation]
+ nice, nohup, and su now refuse to execute the subsidiary program if
+ they detect write failure in printing an otherwise non-fatal warning
+ message to stderr.
+
stat -f recognizes more file system types: afs, cifs, anon-inode FS,
btrfs, cgroupfs, cramfs-wend, debugfs, futexfs, hfs, inotifyfs, minux3,
nilfs, securityfs, selinux, xenfs
diff --git a/src/nice.c b/src/nice.c
index e157db801..a16066be8 100644
--- a/src/nice.c
+++ b/src/nice.c
@@ -185,8 +185,17 @@ main (int argc, char **argv)
ok = (setpriority (PRIO_PROCESS, 0, current_niceness + adjustment) == 0);
#endif
if (!ok)
- error (perm_related_errno (errno) ? 0
- : EXIT_CANCELED, errno, _("cannot set niceness"));
+ {
+ error (perm_related_errno (errno) ? 0
+ : EXIT_CANCELED, errno, _("cannot set niceness"));
+ /* error() flushes stderr, but does not check for write failure.
+ Normally, we would catch this via our atexit() hook of
+ close_stdout, but execvp() gets in the way. If stderr
+ encountered a write failure, there is no need to try calling
+ error() again. */
+ if (ferror (stderr))
+ exit (EXIT_CANCELED);
+ }
execvp (argv[i], &argv[i]);
diff --git a/src/nohup.c b/src/nohup.c
index 880cc7492..1f92c3f5a 100644
--- a/src/nohup.c
+++ b/src/nohup.c
@@ -203,6 +203,15 @@ main (int argc, char **argv)
close (out_fd);
}
+ /* error() flushes stderr, but does not check for write failure.
+ Normally, we would catch this via our atexit() hook of
+ close_stdout, but execvp() gets in the way. If stderr
+ encountered a write failure, there is no need to try calling
+ error() again, particularly since we may have just changed the
+ underlying fd out from under stderr. */
+ if (ferror (stderr))
+ exit (exit_internal_failure);
+
signal (SIGHUP, SIG_IGN);
{
diff --git a/src/su.c b/src/su.c
index add100aeb..694f62eda 100644
--- a/src/su.c
+++ b/src/su.c
@@ -506,5 +506,13 @@ main (int argc, char **argv)
if (simulate_login && chdir (pw->pw_dir) != 0)
error (0, errno, _("warning: cannot change directory to %s"), pw->pw_dir);
+ /* error() flushes stderr, but does not check for write failure.
+ Normally, we would catch this via our atexit() hook of
+ close_stdout, but execv() gets in the way. If stderr
+ encountered a write failure, there is no need to try calling
+ error() again. */
+ if (ferror (stderr))
+ exit (EXIT_CANCELED);
+
run_shell (shell, command, argv + optind, MAX (0, argc - optind));
}
diff --git a/tests/misc/nice b/tests/misc/nice
index cf4d96b23..f85666e34 100755
--- a/tests/misc/nice
+++ b/tests/misc/nice
@@ -82,6 +82,12 @@ if test x`nice -n -1 nice 2> /dev/null` = x0 ; then
mv err exp || framework_failure
nice --1 true 2> err || fail=1
compare exp err || fail=1
+ # Failure to write advisory message is fatal. Buggy through coreutils 8.0.
+ if test -w /dev/full && test -c /dev/full; then
+ nice -n -1 nice > out 2> /dev/full
+ test $? = 125 || fail=1
+ test -s out && fail=1
+ fi
else
# superuser - change succeeds
nice -n -1 nice 2> err || fail=1
diff --git a/tests/misc/nohup b/tests/misc/nohup
index ad04a1cb0..96810588e 100755
--- a/tests/misc/nohup
+++ b/tests/misc/nohup
@@ -64,6 +64,21 @@ test -f nohup.out && fail=1
rm -f nohup.out err
# ----------------------
+# Bug present through coreutils 8.0: failure to print advisory message
+# to stderr must be fatal. Requires stdout to be terminal.
+if test -w /dev/full && test -c /dev/full; then
+(
+ exec >/dev/tty
+ test -t 1 || exit 0
+ nohup echo hi 2> /dev/full
+ test $? = 125 || fail=1
+ test -f nohup.out || fail=1
+ test -s nohup.out && fail=1
+ rm -f nohup.out
+ exit $fail
+) || fail=1
+fi
+
nohup no-such-command 2> err
errno=$?
if test -t 1; then