diff options
| author | Denys Vlasenko <vda.linux@googlemail.com> | 2018-07-24 18:01:22 +0200 |
|---|---|---|
| committer | Denys Vlasenko <vda.linux@googlemail.com> | 2018-07-24 18:01:22 +0200 |
| commit | 945e9b05c910d3b946a5d5a396c5a7e075513e6e (patch) | |
| tree | 9e0dd8e37ca6eded1849791097846ee8e7e32c51 /shell/hush.c | |
| parent | 63c42afaa43d42def05dfbca1f4e10c7314b1f77 (diff) | |
| download | busybox-w32-945e9b05c910d3b946a5d5a396c5a7e075513e6e.tar.gz busybox-w32-945e9b05c910d3b946a5d5a396c5a7e075513e6e.tar.bz2 busybox-w32-945e9b05c910d3b946a5d5a396c5a7e075513e6e.zip | |
hush: fix/explain corner cases of redirection colliding with script fd
function old new delta
save_fd_on_redirect 200 208 +8
run_pipe 1870 1873 +3
setup_redirects 321 322 +1
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 3/0 up/down: 12/0) Total: 12 bytes
Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
Diffstat (limited to 'shell/hush.c')
| -rw-r--r-- | shell/hush.c | 61 |
1 files changed, 49 insertions, 12 deletions
diff --git a/shell/hush.c b/shell/hush.c index 7a1513c24..e3c6e2de9 100644 --- a/shell/hush.c +++ b/shell/hush.c | |||
| @@ -7470,16 +7470,54 @@ static int save_fd_on_redirect(int fd, int avoid_fd, struct squirrel **sqp) | |||
| 7470 | return 1; /* "we closed fd" */ | 7470 | return 1; /* "we closed fd" */ |
| 7471 | } | 7471 | } |
| 7472 | #endif | 7472 | #endif |
| 7473 | /* Are we called from setup_redirects(squirrel==NULL) | ||
| 7474 | * in redirect in a [v]forked child? | ||
| 7475 | */ | ||
| 7476 | if (sqp == NULL) { | ||
| 7477 | /* No need to move script fds. | ||
| 7478 | * For NOMMU case, it's actively wrong: we'd change ->fd | ||
| 7479 | * fields in memory for the parent, but parent's fds | ||
| 7480 | * aren't be moved, it would use wrong fd! | ||
| 7481 | * Reproducer: "cmd 3>FILE" in script. | ||
| 7482 | * If we would call move_HFILEs_on_redirect(), child would: | ||
| 7483 | * fcntl64(3, F_DUPFD_CLOEXEC, 10) = 10 | ||
| 7484 | * close(3) = 0 | ||
| 7485 | * and change ->fd to 10 if fd#3 is a script fd. WRONG. | ||
| 7486 | */ | ||
| 7487 | //bb_error_msg("sqp == NULL: [v]forked child"); | ||
| 7488 | return 0; | ||
| 7489 | } | ||
| 7490 | |||
| 7473 | /* If this one of script's fds? */ | 7491 | /* If this one of script's fds? */ |
| 7474 | if (move_HFILEs_on_redirect(fd, avoid_fd)) | 7492 | if (move_HFILEs_on_redirect(fd, avoid_fd)) |
| 7475 | return 1; /* yes. "we closed fd" (actually moved it) */ | 7493 | return 1; /* yes. "we closed fd" (actually moved it) */ |
| 7476 | 7494 | ||
| 7477 | /* Are we called from setup_redirects(squirrel==NULL)? Two cases: | 7495 | /* Are we called for "exec 3>FILE"? Came through |
| 7478 | * (1) Redirect in a forked child. | 7496 | * redirect_and_varexp_helper(squirrel=ERR_PTR) -> setup_redirects(ERR_PTR) |
| 7479 | * (2) "exec 3>FILE". | 7497 | * This case used to fail for this script: |
| 7498 | * exec 3>FILE | ||
| 7499 | * echo Ok | ||
| 7500 | * ...100000 more lines... | ||
| 7501 | * echo Ok | ||
| 7502 | * as follows: | ||
| 7503 | * read(3, "exec 3>FILE\necho Ok\necho Ok"..., 1024) = 1024 | ||
| 7504 | * open("FILE", O_WRONLY|O_CREAT|O_TRUNC|O_LARGEFILE, 0666) = 4 | ||
| 7505 | * dup2(4, 3) = 3 | ||
| 7506 | * ^^^^^^^^ oops, we lost fd#3 opened to our script! | ||
| 7507 | * close(4) = 0 | ||
| 7508 | * write(1, "Ok\n", 3) = 3 | ||
| 7509 | * ... = 3 | ||
| 7510 | * write(1, "Ok\n", 3) = 3 | ||
| 7511 | * read(3, 0x94fbc08, 1024) = -1 EBADF (Bad file descriptor) | ||
| 7512 | * ^^^^^^^^ oops, wrong fd!!! | ||
| 7513 | * With this case separate from sqp == NULL and *after* move_HFILEs, | ||
| 7514 | * it now works: | ||
| 7480 | */ | 7515 | */ |
| 7481 | if (!sqp) | 7516 | if (sqp == ERR_PTR) { |
| 7517 | /* Don't preserve redirected fds: exec is _meant_ to change these */ | ||
| 7518 | //bb_error_msg("sqp == ERR_PTR: exec >FILE"); | ||
| 7482 | return 0; | 7519 | return 0; |
| 7520 | } | ||
| 7483 | 7521 | ||
| 7484 | /* Check whether it collides with any open fds (e.g. stdio), save fds as needed */ | 7522 | /* Check whether it collides with any open fds (e.g. stdio), save fds as needed */ |
| 7485 | *sqp = add_squirrel(*sqp, fd, avoid_fd); | 7523 | *sqp = add_squirrel(*sqp, fd, avoid_fd); |
| @@ -7618,7 +7656,7 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) | |||
| 7618 | */ | 7656 | */ |
| 7619 | } else { | 7657 | } else { |
| 7620 | /* if newfd is a script fd or saved fd, simulate EBADF */ | 7658 | /* if newfd is a script fd or saved fd, simulate EBADF */ |
| 7621 | if (internally_opened_fd(newfd, sqp ? *sqp : NULL)) { | 7659 | if (internally_opened_fd(newfd, sqp && sqp != ERR_PTR ? *sqp : NULL)) { |
| 7622 | //errno = EBADF; | 7660 | //errno = EBADF; |
| 7623 | //bb_perror_msg_and_die("can't duplicate file descriptor"); | 7661 | //bb_perror_msg_and_die("can't duplicate file descriptor"); |
| 7624 | newfd = -1; /* same effect as code above */ | 7662 | newfd = -1; /* same effect as code above */ |
| @@ -8633,8 +8671,8 @@ static int checkjobs_and_fg_shell(struct pipe *fg_pipe) | |||
| 8633 | * subshell: ( list ) [&] | 8671 | * subshell: ( list ) [&] |
| 8634 | */ | 8672 | */ |
| 8635 | #if !ENABLE_HUSH_MODE_X | 8673 | #if !ENABLE_HUSH_MODE_X |
| 8636 | #define redirect_and_varexp_helper(command, squirrel, argv_expanded) \ | 8674 | #define redirect_and_varexp_helper(command, sqp, argv_expanded) \ |
| 8637 | redirect_and_varexp_helper(command, squirrel) | 8675 | redirect_and_varexp_helper(command, sqp) |
| 8638 | #endif | 8676 | #endif |
| 8639 | static int redirect_and_varexp_helper( | 8677 | static int redirect_and_varexp_helper( |
| 8640 | struct command *command, | 8678 | struct command *command, |
| @@ -8651,10 +8689,6 @@ static int redirect_and_varexp_helper( | |||
| 8651 | /* this takes ownership of new_env[i] elements, and frees new_env: */ | 8689 | /* this takes ownership of new_env[i] elements, and frees new_env: */ |
| 8652 | set_vars_and_save_old(new_env); | 8690 | set_vars_and_save_old(new_env); |
| 8653 | 8691 | ||
| 8654 | /* setup_redirects acts on file descriptors, not FILEs. | ||
| 8655 | * This is perfect for work that comes after exec(). | ||
| 8656 | * Is it really safe for inline use? Experimentally, | ||
| 8657 | * things seem to work. */ | ||
| 8658 | return setup_redirects(command, sqp); | 8692 | return setup_redirects(command, sqp); |
| 8659 | } | 8693 | } |
| 8660 | static NOINLINE int run_pipe(struct pipe *pi) | 8694 | static NOINLINE int run_pipe(struct pipe *pi) |
| @@ -8855,7 +8889,10 @@ static NOINLINE int run_pipe(struct pipe *pi) | |||
| 8855 | */ | 8889 | */ |
| 8856 | enter_var_nest_level(); | 8890 | enter_var_nest_level(); |
| 8857 | G.shadowed_vars_pp = &old_vars; | 8891 | G.shadowed_vars_pp = &old_vars; |
| 8858 | rcode = redirect_and_varexp_helper(command, /*squirrel:*/ NULL, argv_expanded); | 8892 | rcode = redirect_and_varexp_helper(command, |
| 8893 | /*squirrel:*/ ERR_PTR, | ||
| 8894 | argv_expanded | ||
| 8895 | ); | ||
| 8859 | G.shadowed_vars_pp = sv_shadowed; | 8896 | G.shadowed_vars_pp = sv_shadowed; |
| 8860 | /* rcode=1 can be if redir file can't be opened */ | 8897 | /* rcode=1 can be if redir file can't be opened */ |
| 8861 | 8898 | ||