summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTavian Barnes <tavianator@tavianator.com>2019-01-31 23:46:56 -0500
committerTavian Barnes <tavianator@tavianator.com>2019-02-01 12:50:29 -0500
commitaa5f0f5745e4e7392c078d1fe28825c941f52f7c (patch)
tree897376cebf759a6e90c4b94f7b3dd5fe3a42025a
parent02132c2efb6daa213aa07b805ccb0c20241ca2f7 (diff)
downloadbfs-aa5f0f5745e4e7392c078d1fe28825c941f52f7c.tar.xz
main: Fix closed standard stream handling
bfs >&- should complain about a missing file descriptor, rather than silently succeeding.
-rw-r--r--main.c40
-rwxr-xr-xtests.sh29
-rw-r--r--tests/test_closed_stdin.out19
-rw-r--r--tests/test_ok_closed_stdin.out0
-rw-r--r--tests/test_okdir_closed_stdin.out0
-rw-r--r--util.c10
6 files changed, 76 insertions, 22 deletions
diff --git a/main.c b/main.c
index 2734aaf..3c19092 100644
--- a/main.c
+++ b/main.c
@@ -1,6 +1,6 @@
/****************************************************************************
* bfs *
- * Copyright (C) 2015-2017 Tavian Barnes <tavianator@tavianator.com> *
+ * Copyright (C) 2015-2019 Tavian Barnes <tavianator@tavianator.com> *
* *
* Permission to use, copy, modify, and/or distribute this software for any *
* purpose with or without fee is hereby granted. *
@@ -24,16 +24,31 @@
#include <unistd.h>
/**
- * Ensure that a file descriptor is open.
+ * Make sure the standard streams std{in,out,err} are open. If they are not,
+ * future open() calls may use those file descriptors, and std{in,out,err} will
+ * use them unintentionally.
*/
-static int ensure_fd_open(int fd, int flags) {
- if (isopen(fd)) {
- return 0;
- } else if (redirect(fd, "/dev/null", flags) >= 0) {
- return 0;
- } else {
+static int open_std_streams() {
+#ifdef O_PATH
+ const int inflags = O_PATH, outflags = O_PATH;
+#else
+ // These are intentionally backwards so that bfs >&- still fails with EBADF
+ const int inflags = O_WRONLY, outflags = O_RDONLY;
+#endif
+
+ if (!isopen(STDERR_FILENO) && redirect(STDERR_FILENO, "/dev/null", outflags) < 0) {
+ return -1;
+ }
+ if (!isopen(STDOUT_FILENO) && redirect(STDOUT_FILENO, "/dev/null", outflags) < 0) {
+ perror("redirect()");
+ return -1;
+ }
+ if (!isopen(STDIN_FILENO) && redirect(STDIN_FILENO, "/dev/null", inflags) < 0) {
+ perror("redirect()");
return -1;
}
+
+ return 0;
}
/**
@@ -42,13 +57,8 @@ static int ensure_fd_open(int fd, int flags) {
int main(int argc, char *argv[]) {
int ret = EXIT_FAILURE;
- if (ensure_fd_open(STDIN_FILENO, O_RDONLY) != 0) {
- goto done;
- }
- if (ensure_fd_open(STDOUT_FILENO, O_WRONLY) != 0) {
- goto done;
- }
- if (ensure_fd_open(STDERR_FILENO, O_WRONLY) != 0) {
+ // Make sure the standard streams are open
+ if (open_std_streams() != 0) {
goto done;
}
diff --git a/tests.sh b/tests.sh
index 775a254..302a4c2 100755
--- a/tests.sh
+++ b/tests.sh
@@ -290,6 +290,11 @@ posix_tests=(
test_user_name
test_user_id
+ # Closed file descriptors
+ test_closed_stdin
+ test_closed_stdout
+ test_closed_stderr
+
# PATH_MAX handling
test_deep
@@ -385,8 +390,10 @@ bsd_tests=(
test_nouser
test_ok_stdin
+ test_ok_closed_stdin
test_okdir_stdin
+ test_okdir_closed_stdin
test_perm_000_plus
test_perm_222_plus
@@ -525,8 +532,10 @@ gnu_tests=(
test_nouser
+ test_ok_closed_stdin
test_ok_nothing
+ test_okdir_closed_stdin
test_okdir_plus_semicolon
test_perm_000_slash
@@ -1929,6 +1938,26 @@ function test_fprint_error() {
fi
}
+function test_closed_stdin() {
+ bfs_diff basic <&-
+}
+
+function test_ok_closed_stdin() {
+ bfs_diff basic -ok echo \; <&- 2>/dev/null
+}
+
+function test_okdir_closed_stdin() {
+ bfs_diff basic -okdir echo {} \; <&- 2>/dev/null
+}
+
+function test_closed_stdout() {
+ ! invoke_bfs basic >&- 2>/dev/null
+}
+
+function test_closed_stderr() {
+ ! invoke_bfs basic >&- 2>&-
+}
+
if [ -t 1 -a ! "$VERBOSE" ]; then
in_place=yes
fi
diff --git a/tests/test_closed_stdin.out b/tests/test_closed_stdin.out
new file mode 100644
index 0000000..bb3cd8d
--- /dev/null
+++ b/tests/test_closed_stdin.out
@@ -0,0 +1,19 @@
+basic
+basic/a
+basic/b
+basic/c
+basic/e
+basic/g
+basic/i
+basic/j
+basic/k
+basic/l
+basic/c/d
+basic/e/f
+basic/g/h
+basic/j/foo
+basic/k/foo
+basic/l/foo
+basic/k/foo/bar
+basic/l/foo/bar
+basic/l/foo/bar/baz
diff --git a/tests/test_ok_closed_stdin.out b/tests/test_ok_closed_stdin.out
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/test_ok_closed_stdin.out
diff --git a/tests/test_okdir_closed_stdin.out b/tests/test_okdir_closed_stdin.out
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/test_okdir_closed_stdin.out
diff --git a/util.c b/util.c
index b708527..1e5ebdb 100644
--- a/util.c
+++ b/util.c
@@ -85,8 +85,6 @@ bool isopen(int fd) {
}
int redirect(int fd, const char *path, int flags, ...) {
- close(fd);
-
mode_t mode = 0;
if (flags & O_CREAT) {
va_list args;
@@ -102,11 +100,9 @@ int redirect(int fd, const char *path, int flags, ...) {
int ret = open(path, flags, mode);
if (ret >= 0 && ret != fd) {
- int other = ret;
- ret = dup2(other, fd);
- if (close(other) != 0) {
- ret = -1;
- }
+ int orig = ret;
+ ret = dup2(orig, fd);
+ close(orig);
}
return ret;