[git commit] hush: do not exit interactive shell on some redirection errors
Denys Vlasenko
vda.linux at googlemail.com
Sat Jul 13 00:13:28 UTC 2024
commit: https://git.busybox.net/busybox/commit/?id=23da5c4b716b92524240c6f81c2e2474c1825cfc
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master
$ echo >&99
hush: dup2(99,1): Bad file descriptor
$ echo >&9999
hush: fcntl(1,F_DUPFD,10000): Invalid argument
$ echo 2>/dev/tty 10>&9999
hush: fcntl(10,F_DUPFD,10000): Invalid argument
$ still alive!_
function old new delta
static.setup_redirects 334 394 +60
.rodata 105661 105712 +51
dup_CLOEXEC 49 79 +30
save_fd_on_redirect 263 277 +14
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 4/0 up/down: 155/0) Total: 155 bytes
Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
shell/ash_test/ash-redir/redir_exec2.tests | 1 -
shell/hush.c | 51 +++++++++++++++++-----
shell/hush_test/hush-redir/redir3.right | 3 +-
shell/hush_test/hush-redir/redir_exec2.tests | 1 -
shell/hush_test/hush-redir/redir_to_bad_fd.right | 3 +-
.../hush_test/hush-redir/redir_to_bad_fd255.right | 3 +-
shell/hush_test/hush-redir/redir_to_bad_fd3.right | 3 +-
7 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/shell/ash_test/ash-redir/redir_exec2.tests b/shell/ash_test/ash-redir/redir_exec2.tests
index 700a51786..0af767592 100755
--- a/shell/ash_test/ash-redir/redir_exec2.tests
+++ b/shell/ash_test/ash-redir/redir_exec2.tests
@@ -8,4 +8,3 @@ ls -1 fd/3
exec 5>&-
test -e fd/5 && echo BUG
echo One:$?
-
diff --git a/shell/hush.c b/shell/hush.c
index 1d5642260..707b77c9c 100644
--- a/shell/hush.c
+++ b/shell/hush.c
@@ -1581,6 +1581,16 @@ static int dup_CLOEXEC(int fd, int avoid_fd)
goto repeat;
if (errno == EINTR)
goto repeat;
+ if (errno != EBADF) {
+ /* "echo >&9999" gets EINVAL trying to save fd 1 to above 9999.
+ * We could try saving it _below_ 9999 instead (how?), but
+ * this probably means that dup2(9999,1) to effectuate >&9999
+ * would also not work: fd 9999 can't exist.
+ * (This differs from "echo >&99" where saving works, but
+ * subsequent dup2(99,1) fails if fd 99 is not open).
+ */
+ bb_perror_msg("fcntl(%d,F_DUPFD,%d)", fd, avoid_fd + 1);
+ }
}
return newfd;
}
@@ -7893,10 +7903,16 @@ static struct squirrel *add_squirrel(struct squirrel *sq, int fd, int avoid_fd)
if (sq) for (; sq[i].orig_fd >= 0; i++) {
/* If we collide with an already moved fd... */
if (fd == sq[i].moved_to) {
- sq[i].moved_to = dup_CLOEXEC(sq[i].moved_to, avoid_fd);
- debug_printf_redir("redirect_fd %d: already busy, moving to %d\n", fd, sq[i].moved_to);
- if (sq[i].moved_to < 0) /* what? */
- xfunc_die();
+ moved_to = dup_CLOEXEC(sq[i].moved_to, avoid_fd);
+ debug_printf_redir("redirect_fd %d: already busy, moving to %d\n", fd, moved_to);
+ if (moved_to < 0) {
+ /* "echo 2>/dev/tty 10>&9999" testcase:
+ * We move fd 2 to 10, then discover we need to move fd 10
+ * (and not hit 9999) and the latter fails.
+ */
+ return NULL; /* fcntl failed */
+ }
+ sq[i].moved_to = moved_to;
return sq;
}
if (fd == sq[i].orig_fd) {
@@ -7910,7 +7926,7 @@ static struct squirrel *add_squirrel(struct squirrel *sq, int fd, int avoid_fd)
moved_to = dup_CLOEXEC(fd, avoid_fd);
debug_printf_redir("redirect_fd %d: previous fd is moved to %d (-1 if it was closed)\n", fd, moved_to);
if (moved_to < 0 && errno != EBADF)
- xfunc_die();
+ return NULL; /* fcntl failed (not because fd is closed) */
return append_squirrel(sq, i, fd, moved_to);
}
@@ -7943,6 +7959,8 @@ static struct squirrel *add_squirrel_closed(struct squirrel *sq, int fd)
*/
static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp)
{
+ struct squirrel *new_squirrel;
+
if (avoid_fd < 9) /* the important case here is that it can be -1 */
avoid_fd = 9;
@@ -8006,7 +8024,10 @@ static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp)
}
/* Check whether it collides with any open fds (e.g. stdio), save fds as needed */
- *sqp = add_squirrel(*sqp, fd, avoid_fd);
+ new_squirrel = add_squirrel(*sqp, fd, avoid_fd);
+ if (!new_squirrel)
+ return -1; /* redirect error */
+ *sqp = new_squirrel;
return 0; /* "we did not close fd" */
}
@@ -8092,7 +8113,8 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp)
if (redir->rd_type == REDIRECT_HEREDOC2) {
/* "rd_fd<<HERE" case */
- save_fd_on_redirect(redir->rd_fd, /*avoid:*/ 0, sqp);
+ if (save_fd_on_redirect(redir->rd_fd, /*avoid:*/ 0, sqp) < 0)
+ return 1;
/* for REDIRECT_HEREDOC2, rd_filename holds _contents_
* of the heredoc */
debug_printf_redir("set heredoc '%s'\n",
@@ -8151,6 +8173,8 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp)
/* if "N>&-": close redir->rd_fd (newfd is REDIRFD_CLOSE) */
closed = save_fd_on_redirect(redir->rd_fd, /*avoid:*/ newfd, sqp);
+ if (closed < 0)
+ return 1; /* error */
if (newfd == REDIRFD_CLOSE) {
/* "N>&-" means "close me" */
if (!closed) {
@@ -8164,13 +8188,16 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp)
* and second redirect closes 3! Restore code then closes 3 again.
*/
} else {
- /* if newfd is a script fd or saved fd, simulate EBADF */
+ /* if newfd is a script fd or saved fd, do not allow to use it */
if (internally_opened_fd(newfd, sqp && sqp != ERR_PTR ? *sqp : NULL)) {
- //errno = EBADF;
- //bb_perror_msg_and_die("can't duplicate file descriptor");
- newfd = -1; /* same effect as code above */
+ bb_error_msg("fd#%d is not open", newfd);
+ return 1;
+ }
+ if (dup2(newfd, redir->rd_fd) < 0) {
+ /* "echo >&99" testcase */
+ bb_perror_msg("dup2(%d,%d)", newfd, redir->rd_fd);
+ return 1;
}
- xdup2(newfd, redir->rd_fd);
if (redir->rd_dup == REDIRFD_TO_FILE)
/* "rd_fd > FILE" */
close(newfd);
diff --git a/shell/hush_test/hush-redir/redir3.right b/shell/hush_test/hush-redir/redir3.right
index e3c878b7d..d49237c42 100644
--- a/shell/hush_test/hush-redir/redir3.right
+++ b/shell/hush_test/hush-redir/redir3.right
@@ -1,2 +1,3 @@
TEST
-hush: can't duplicate file descriptor: Bad file descriptor
+hush: dup2(9,1): Bad file descriptor
+Output to fd#9: 1
diff --git a/shell/hush_test/hush-redir/redir_exec2.tests b/shell/hush_test/hush-redir/redir_exec2.tests
index 700a51786..0af767592 100755
--- a/shell/hush_test/hush-redir/redir_exec2.tests
+++ b/shell/hush_test/hush-redir/redir_exec2.tests
@@ -8,4 +8,3 @@ ls -1 fd/3
exec 5>&-
test -e fd/5 && echo BUG
echo One:$?
-
diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd.right b/shell/hush_test/hush-redir/redir_to_bad_fd.right
index 936911ce5..4fc51cc93 100644
--- a/shell/hush_test/hush-redir/redir_to_bad_fd.right
+++ b/shell/hush_test/hush-redir/redir_to_bad_fd.right
@@ -1 +1,2 @@
-hush: can't duplicate file descriptor: Bad file descriptor
+hush: dup2(10,1): Bad file descriptor
+OK
diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd255.right b/shell/hush_test/hush-redir/redir_to_bad_fd255.right
index 936911ce5..baa2959ca 100644
--- a/shell/hush_test/hush-redir/redir_to_bad_fd255.right
+++ b/shell/hush_test/hush-redir/redir_to_bad_fd255.right
@@ -1 +1,2 @@
-hush: can't duplicate file descriptor: Bad file descriptor
+hush: dup2(255,1): Bad file descriptor
+OK
diff --git a/shell/hush_test/hush-redir/redir_to_bad_fd3.right b/shell/hush_test/hush-redir/redir_to_bad_fd3.right
index 936911ce5..5e54abc0a 100644
--- a/shell/hush_test/hush-redir/redir_to_bad_fd3.right
+++ b/shell/hush_test/hush-redir/redir_to_bad_fd3.right
@@ -1 +1,2 @@
-hush: can't duplicate file descriptor: Bad file descriptor
+hush: fd#3 is not open
+OK
More information about the busybox-cvs
mailing list