From 05e5d6a38157cabc733fc30acf3646a099cf139c Mon Sep 17 00:00:00 2001 From: Petja Patjas Date: Mon, 17 Apr 2023 16:28:53 +0300 Subject: vi: Ensure that the edit buffer ends in a newline Currently vi assumes that the edit buffer ends in a newline. This may not be the case. For example: $ printf test > test $ vi test We fix this by inserting a newline to the end during initialization. Signed-off-by: Petja Patjas Signed-off-by: Denys Vlasenko --- editors/vi.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/editors/vi.c b/editors/vi.c index 3cc3d2a0b..34932f60c 100644 --- a/editors/vi.c +++ b/editors/vi.c @@ -2315,9 +2315,10 @@ static int init_text_buffer(char *fn) update_filename(fn); rc = file_insert(fn, text, 1); - if (rc < 0) { - // file doesnt exist. Start empty buf with dummy line - char_insert(text, '\n', NO_UNDO); + if (rc <= 0 || *(end - 1) != '\n') { + // file doesn't exist or doesn't end in a newline. + // insert a newline to the end + char_insert(end, '\n', NO_UNDO); } flush_undo_data(); -- cgit v1.2.3-55-g6feb From a3c50683069c797ef328f6e32c6282d5d3243c98 Mon Sep 17 00:00:00 2001 From: Ron Yorston Date: Wed, 3 Apr 2024 13:59:25 +0100 Subject: md5/shaXsum: accept uppercase hex strings The coreutils versions of md5sum and the like accept uppercase hex strings from checksum files specified with the '-c' option. Use a case-insensitive comparison so BusyBox does the same. Signed-off-by: Ron Yorston Signed-off-by: Denys Vlasenko --- coreutils/md5_sha1_sum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coreutils/md5_sha1_sum.c b/coreutils/md5_sha1_sum.c index f6a21237d..978d328f1 100644 --- a/coreutils/md5_sha1_sum.c +++ b/coreutils/md5_sha1_sum.c @@ -320,7 +320,7 @@ int md5_sha1_sum_main(int argc UNUSED_PARAM, char **argv) hash_value = hash_file(in_buf, filename_ptr, sha3_width); - if (hash_value && (strcmp((char*)hash_value, line) == 0)) { + if (hash_value && (strcasecmp((char*)hash_value, line) == 0)) { if (!(flags & FLAG_SILENT)) printf("%s: OK\n", filename_ptr); } else { -- cgit v1.2.3-55-g6feb From d745852f136bac4646e50a4f03565273e687b28b Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Thu, 11 Jul 2024 23:48:53 +0200 Subject: tls: P256: fix obscure x86_64 asm misbehavior, closes 15679 gcc does not necessarily clear upper bits in 64-bit regs if you ask it to load a 32-bit constant. Cast it to unsigned long. Better yet, hand-write loading of the constant with a smaller instruction. Signed-off-by: Denys Vlasenko --- networking/tls_sp_c32.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/networking/tls_sp_c32.c b/networking/tls_sp_c32.c index a593c5c40..9ab996f3b 100644 --- a/networking/tls_sp_c32.c +++ b/networking/tls_sp_c32.c @@ -425,26 +425,45 @@ static void sp_256_sub_8_p256_mod(sp_digit* r) #elif ALLOW_ASM && defined(__GNUC__) && defined(__x86_64__) static void sp_256_sub_8_p256_mod(sp_digit* r) { +//p256_mod[3..0] = ffffffff00000001 0000000000000000 00000000ffffffff ffffffffffffffff +# if 0 + // gcc -Oz bug (?) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115875 + // uses buggy "push $-1; pop %rax" insns to load 00000000ffffffff uint64_t reg; uint64_t ooff; -//p256_mod[3..0] = ffffffff00000001 0000000000000000 00000000ffffffff ffffffffffffffff asm volatile ( -"\n addq $1, (%0)" // adding 1 is the same as subtracting ffffffffffffffff -"\n cmc" // only carry bit needs inverting -"\n" -"\n sbbq %1, 1*8(%0)" // %1 holds 00000000ffffffff -"\n" +"\n subq $0xffffffffffffffff, (%0)" +"\n sbbq %1, 1*8(%0)" "\n sbbq $0, 2*8(%0)" -"\n" "\n movq 3*8(%0), %2" -"\n sbbq $0, %2" // adding 00000000ffffffff (in %1) -"\n addq %1, %2" // is the same as subtracting ffffffff00000001 +"\n sbbq $0, %2" // subtract carry +"\n addq %1, %2" // adding 00000000ffffffff (in %1) +"\n" // is the same as subtracting ffffffff00000001 "\n movq %2, 3*8(%0)" "\n" : "=r" (r), "=r" (ooff), "=r" (reg) - : "0" (r), "1" (0x00000000ffffffff) + : "0" (r), "1" (0x00000000ffffffffUL) /* UL is important! */ + : "memory" + ); +# else // let's do it by hand: + uint64_t reg; + uint64_t rax; + asm volatile ( +"\n orl $0xffffffff, %%eax" // %1 (rax) = 00000000ffffffff +"\n subq $0xffffffffffffffff, (%0)" +"\n sbbq %1, 1*8(%0)" +"\n sbbq $0, 2*8(%0)" +"\n movq 3*8(%0), %2" +"\n sbbq $0, %2" // subtract carry +"\n addq %1, %2" // adding 00000000ffffffff (in %1) +"\n" // is the same as subtracting ffffffff00000001 +"\n movq %2, 3*8(%0)" +"\n" + : "=r" (r), "=&a" (rax), "=r" (reg) + : "0" (r) : "memory" ); +# endif } #else static void sp_256_sub_8_p256_mod(sp_digit* r) -- cgit v1.2.3-55-g6feb From 999e290ef64cbd49a9e0a0f6d3cfaf26414c1c3e Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 12 Jul 2024 11:08:08 +0200 Subject: tls: P256: improve x86_64 multiplication asm code gcc is being rather silly. Usues suboptimal registers, and does not realize that i and j are never negative, thus usese even _more_ registers for temporaries to sign-extend i/j to 64-bit offsets. function old new delta sp_256_mont_mul_8 155 132 -23 Signed-off-by: Denys Vlasenko --- networking/tls_sp_c32.c | 58 ++++++++++++++++++++++++++++++------------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/networking/tls_sp_c32.c b/networking/tls_sp_c32.c index 9ab996f3b..e493c436a 100644 --- a/networking/tls_sp_c32.c +++ b/networking/tls_sp_c32.c @@ -411,10 +411,10 @@ static void sp_256_sub_8_p256_mod(sp_digit* r) "\n subl $0xffffffff, (%0)" "\n sbbl $0xffffffff, 1*4(%0)" "\n sbbl $0xffffffff, 2*4(%0)" -"\n sbbl $0, 3*4(%0)" -"\n sbbl $0, 4*4(%0)" -"\n sbbl $0, 5*4(%0)" -"\n sbbl $1, 6*4(%0)" +"\n sbbl $0x00000000, 3*4(%0)" +"\n sbbl $0x00000000, 4*4(%0)" +"\n sbbl $0x00000000, 5*4(%0)" +"\n sbbl $0x00000001, 6*4(%0)" "\n sbbl $0xffffffff, 7*4(%0)" "\n" : "=r" (r) @@ -433,10 +433,10 @@ static void sp_256_sub_8_p256_mod(sp_digit* r) uint64_t ooff; asm volatile ( "\n subq $0xffffffffffffffff, (%0)" -"\n sbbq %1, 1*8(%0)" -"\n sbbq $0, 2*8(%0)" +"\n sbbq %1, 1*8(%0)" // %1 = 00000000ffffffff +"\n sbbq $0x0000000000000000, 2*8(%0)" "\n movq 3*8(%0), %2" -"\n sbbq $0, %2" // subtract carry +"\n sbbq $0x0, %2" // subtract carry "\n addq %1, %2" // adding 00000000ffffffff (in %1) "\n" // is the same as subtracting ffffffff00000001 "\n movq %2, 3*8(%0)" @@ -452,9 +452,9 @@ static void sp_256_sub_8_p256_mod(sp_digit* r) "\n orl $0xffffffff, %%eax" // %1 (rax) = 00000000ffffffff "\n subq $0xffffffffffffffff, (%0)" "\n sbbq %1, 1*8(%0)" -"\n sbbq $0, 2*8(%0)" +"\n sbbq $0x0000000000000000, 2*8(%0)" "\n movq 3*8(%0), %2" -"\n sbbq $0, %2" // subtract carry +"\n sbbq $0x0, %2" // subtract carry "\n addq %1, %2" // adding 00000000ffffffff (in %1) "\n" // is the same as subtracting ffffffff00000001 "\n movq %2, 3*8(%0)" @@ -495,15 +495,23 @@ static void sp_256to512_mul_8(sp_digit* r, const sp_digit* a, const sp_digit* b) //////////////////////// // uint64_t m = ((uint64_t)a[i]) * b[j]; // acc_hi:acch:accl += m; + long eax_clobbered; asm volatile ( // a[i] is already loaded in %%eax -"\n mull %7" +"\n mull %8" "\n addl %%eax, %0" "\n adcl %%edx, %1" -"\n adcl $0, %2" - : "=rm" (accl), "=rm" (acch), "=rm" (acc_hi) - : "0" (accl), "1" (acch), "2" (acc_hi), "a" (a[i]), "m" (b[j]) +"\n adcl $0x0, %2" + : "=rm" (accl), "=rm" (acch), "=rm" (acc_hi), "=a" (eax_clobbered) + : "0" (accl), "1" (acch), "2" (acc_hi), "3" (a[i]), "m" (b[j]) : "cc", "dx" +// What is "eax_clobbered"? gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html: +// "Do not modify the contents of input-only operands (except for inputs tied +// to outputs). The compiler assumes that on exit from the asm statement these +// operands contain the same values as they had before executing the statement. +// It is not possible to use clobbers to inform the compiler that the values +// in these inputs are changing. One common work-around is to tie the changing +// input variable to an output variable that never gets used." ); //////////////////////// j--; @@ -519,15 +527,20 @@ static void sp_256to512_mul_8(sp_digit* r, const sp_digit* a, const sp_digit* b) const uint64_t* bb = (const void*)b; uint64_t* rr = (void*)r; int k; - uint64_t accl; - uint64_t acch; + register uint64_t accl asm("r8"); + register uint64_t acch asm("r9"); + /* ^^^ ask gcc to not use rax/rdx/input arg regs for accumulator variables */ + /* (or else it may generate lots of silly mov's and even xchg's!) */ acch = accl = 0; for (k = 0; k < 7; k++) { - int i, j; - uint64_t acc_hi; + unsigned i, j; + /* ^^^^^ not signed "int", + * or gcc can use a temp register to sign-extend i,j for aa[i],bb[j] */ + register uint64_t acc_hi asm("r10"); + /* ^^^ ask gcc to not use rax/rdx/input arg regs for accumulators */ i = k - 3; - if (i < 0) + if ((int)i < 0) i = 0; j = k - i; acc_hi = 0; @@ -535,14 +548,15 @@ static void sp_256to512_mul_8(sp_digit* r, const sp_digit* a, const sp_digit* b) //////////////////////// // uint128_t m = ((uint128_t)a[i]) * b[j]; // acc_hi:acch:accl += m; + long rax_clobbered; asm volatile ( // aa[i] is already loaded in %%rax -"\n mulq %7" +"\n mulq %8" "\n addq %%rax, %0" "\n adcq %%rdx, %1" -"\n adcq $0, %2" - : "=rm" (accl), "=rm" (acch), "=rm" (acc_hi) - : "0" (accl), "1" (acch), "2" (acc_hi), "a" (aa[i]), "m" (bb[j]) +"\n adcq $0x0, %2" + : "=rm" (accl), "=rm" (acch), "=rm" (acc_hi), "=a" (rax_clobbered) + : "0" (accl), "1" (acch), "2" (acc_hi), "3" (aa[i]), "m" (bb[j]) : "cc", "dx" ); //////////////////////// -- cgit v1.2.3-55-g6feb From 1ad2f5cd9fe5de0f19212924e100c6d87229c950 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 12 Jul 2024 19:30:14 +0200 Subject: tls: fix CONFIG_FEATURE_TLS_SHA1=y + CONFIG_SHA1_HWACCEL=y The check for result hash size was buggy for CONFIG_SHA1_HWACCEL=y. While at it, document CPUID use a bit better. function old new delta get_shaNI - 28 +28 sha1_end 66 79 +13 sha256_begin 83 60 -23 sha1_begin 111 88 -23 ------------------------------------------------------------------------------ (add/remove: 1/0 grow/shrink: 1/2 up/down: 41/-46) Total: -5 bytes Signed-off-by: Denys Vlasenko --- libbb/hash_md5_sha.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/libbb/hash_md5_sha.c b/libbb/hash_md5_sha.c index 88baf51dc..57a801459 100644 --- a/libbb/hash_md5_sha.c +++ b/libbb/hash_md5_sha.c @@ -15,18 +15,28 @@ #if ENABLE_SHA1_HWACCEL || ENABLE_SHA256_HWACCEL # if defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__)) -static void cpuid(unsigned *eax, unsigned *ebx, unsigned *ecx, unsigned *edx) +static void cpuid_eax_ebx_ecx(unsigned *eax, unsigned *ebx, unsigned *ecx, unsigned *edx) { asm ("cpuid" : "=a"(*eax), "=b"(*ebx), "=c"(*ecx), "=d"(*edx) - : "0"(*eax), "1"(*ebx), "2"(*ecx), "3"(*edx) + : "0" (*eax), "1" (*ebx), "2" (*ecx) ); } static smallint shaNI; -static int get_shaNI(void) +static NOINLINE int get_shaNI(void) { - unsigned eax = 7, ebx = ebx, ecx = 0, edx = edx; - cpuid(&eax, &ebx, &ecx, &edx); + /* Get leaf 7 subleaf 0. Exists on all CPUs since Merom (2006). + * "If a value entered for CPUID.EAX is higher than the maximum + * input value for basic or extended function for that processor + * then the data for the highest basic information leaf is returned". + * This means that Pentiums 4 would return leaf 5 or 6 instead of 7, + * which happen to have zero in EBX bit 29. Thus they should work too. + */ + unsigned eax = 7; + unsigned ecx = 0; + unsigned ebx = 0; /* should not be needed, paranoia */ + unsigned edx; + cpuid_eax_ebx_ecx(&eax, &ebx, &ecx, &edx); ebx = ((ebx >> 28) & 2) - 1; /* bit 29 -> 1 or -1 */ shaNI = (int)ebx; return (int)ebx; @@ -1300,7 +1310,14 @@ unsigned FAST_FUNC sha1_end(sha1_ctx_t *ctx, void *resbuf) /* SHA stores total in BE, need to swap on LE arches: */ common64_end(ctx, /*swap_needed:*/ BB_LITTLE_ENDIAN); - hash_size = (ctx->process_block == sha1_process_block64) ? 5 : 8; + hash_size = 8; + if (ctx->process_block == sha1_process_block64 +#if ENABLE_SHA1_HWACCEL + || ctx->process_block == sha1_process_block64_shaNI +#endif + ) { + hash_size = 5; + } /* This way we do not impose alignment constraints on resbuf: */ if (BB_LITTLE_ENDIAN) { unsigned i; -- cgit v1.2.3-55-g6feb From 0829fce079dae45737330114c27886cb8af85043 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 12 Jul 2024 21:58:04 +0200 Subject: ash: do not abort interactive mode on >&9999 redirect With very large fd#, the error code path is different from one for closed but small fd#. Make it not abort if we are interactive: $ echo text >&99 # this wasn't buggy ash: dup2(9,1): Bad file descriptor $ echo text >&9999 # this was ash: fcntl(1,F_DUPFD,10000): Invalid argument function old new delta .rodata 105637 105661 +24 dup2_or_raise 35 38 +3 redirect 1084 1044 -40 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/1 up/down: 27/-40) Total: -13 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index 094a87390..39455c7c5 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -5621,10 +5621,13 @@ dup2_or_raise(int from, int to) newfd = (from != to) ? dup2(from, to) : to; if (newfd < 0) { /* Happens when source fd is not open: try "echo >&99" */ - ash_msg_and_raise_perror("%d", from); + ash_msg_and_raise_perror("dup2(%d,%d)", from, to); } return newfd; } +/* The only possible error return is EBADF (fd wasn't open). + * Transient errors retry, other errors raise exception. + */ static int dup_CLOEXEC(int fd, int avoid_fd) { @@ -5639,6 +5642,16 @@ 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. Gracefully bail out. + * (This differs from "echo >&99" where saving works, but + * subsequent dup2(99,1) fails if fd 99 is not open). + */ + ash_msg_and_raise_perror("fcntl(%d,F_DUPFD,%d)", fd, avoid_fd + 1); + } } return newfd; } @@ -5754,7 +5767,7 @@ save_fd_on_redirect(int fd, int avoid_fd, struct redirtab *sq) new_fd = dup_CLOEXEC(fd, avoid_fd); sq->two_fd[i].moved_to = new_fd; TRACE(("redirect_fd %d: already busy, moving to %d\n", fd, new_fd)); - if (new_fd < 0) /* what? */ + if (new_fd < 0) /* EBADF? what? */ xfunc_die(); return 0; /* "we did not close fd" */ } @@ -5769,8 +5782,7 @@ save_fd_on_redirect(int fd, int avoid_fd, struct redirtab *sq) new_fd = dup_CLOEXEC(fd, avoid_fd); TRACE(("redirect_fd %d: previous fd is moved to %d (-1 if it was closed)\n", fd, new_fd)); if (new_fd < 0) { - if (errno != EBADF) - xfunc_die(); + /* EBADF (fd is not open) */ /* new_fd = CLOSED; - already is -1 */ } sq->two_fd[i].moved_to = new_fd; -- cgit v1.2.3-55-g6feb From 08fb86726b508eb99502a6681da94395c4e4580c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Fri, 12 Jul 2024 22:28:25 +0200 Subject: ash: remove limitation on fd# length "echo text >&0000000000002" works as you would expect, "echo text >&9999999999" properly fails instead of creating a file named "9999999999". function old new delta expredir 219 232 +13 readtoken1 3045 3053 +8 parsefname 204 201 -3 isdigit_str9 45 - -45 ------------------------------------------------------------------------------ (add/remove: 0/1 grow/shrink: 2/1 up/down: 21/-48) Total: -27 bytes Signed-off-by: Denys Vlasenko --- shell/ash.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/shell/ash.c b/shell/ash.c index 39455c7c5..9da3e956a 100644 --- a/shell/ash.c +++ b/shell/ash.c @@ -557,10 +557,9 @@ static void trace_vprintf(const char *fmt, va_list va); #define is_in_name(c) ((c) == '_' || isalnum((unsigned char)(c))) static int -isdigit_str9(const char *str) +isdigit_str(const char *str) { - int maxlen = 9 + 1; /* max 9 digits: 999999999 */ - while (--maxlen && isdigit(*str)) + while (isdigit(*str)) str++; return (*str == '\0'); } @@ -9661,7 +9660,7 @@ expredir(union node *n) if (fn.list == NULL) ash_msg_and_raise_error("redir error"); #if BASH_REDIR_OUTPUT - if (!isdigit_str9(fn.list->text)) { + if (!isdigit_str(fn.list->text)) { /* >&file, not >&fd */ if (redir->nfile.fd != 1) /* 123>&file - BAD */ ash_msg_and_raise_error("redir error"); @@ -12011,12 +12010,19 @@ fixredir(union node *n, const char *text, int err) if (!err) n->ndup.vname = NULL; + if (LONE_DASH(text)) { + n->ndup.dupfd = -1; + return; + } + fd = bb_strtou(text, NULL, 10); if (!errno && fd >= 0) n->ndup.dupfd = fd; - else if (LONE_DASH(text)) - n->ndup.dupfd = -1; else { + /* This also fails on very large numbers + * which overflow "int" - bb_strtou() does not + * silently truncate results to word width. + */ if (err) raise_error_syntax("bad fd number"); n->ndup.vname = makename(); @@ -12702,7 +12708,7 @@ readtoken1(int c, int syntax, char *eofmark, int striptabs) if ((c == '>' || c == '<' IF_BASH_REDIR_OUTPUT( || c == 0x100 + '>')) && quotef == 0 ) { - if (isdigit_str9(out)) { + if (isdigit_str(out)) { PARSEREDIR(); /* passed as params: out, c */ lasttoken = TREDIR; return lasttoken; -- cgit v1.2.3-55-g6feb From 6c38d0e9da2d4a3defd2bff9f3e00f6229e9824b Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 13 Jul 2024 00:14:41 +0200 Subject: hush: avoid duplicate fcntl(F_SETFD, FD_CLOEXEC) during init function old new delta hush_main 1149 1150 +1 Signed-off-by: Denys Vlasenko --- shell/hush.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/shell/hush.c b/shell/hush.c index 3e6a13b32..afbc3ebec 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -1575,7 +1575,7 @@ static int dup_CLOEXEC(int fd, int avoid_fd) newfd = fcntl(fd, F_DUPFD_CLOEXEC, avoid_fd + 1); if (newfd >= 0) { if (F_DUPFD_CLOEXEC == F_DUPFD) /* if old libc (w/o F_DUPFD_CLOEXEC) */ - fcntl(newfd, F_SETFD, FD_CLOEXEC); + close_on_exec_on(newfd); } else { /* newfd < 0 */ if (errno == EBUSY) goto repeat; @@ -1601,7 +1601,7 @@ static int xdup_CLOEXEC_and_close(int fd, int avoid_fd) xfunc_die(); } if (F_DUPFD_CLOEXEC == F_DUPFD) /* if old libc (w/o F_DUPFD_CLOEXEC) */ - fcntl(newfd, F_SETFD, FD_CLOEXEC); + close_on_exec_on(newfd); close(fd); return newfd; } @@ -10710,7 +10710,7 @@ int hush_main(int argc, char **argv) G_interactive_fd = dup_CLOEXEC(STDIN_FILENO, 254); if (G_interactive_fd < 0) { /* try to dup to any fd */ - G_interactive_fd = dup(STDIN_FILENO); + G_interactive_fd = dup_CLOEXEC(STDIN_FILENO, -1); if (G_interactive_fd < 0) { /* give up */ G_interactive_fd = 0; @@ -10720,8 +10720,6 @@ int hush_main(int argc, char **argv) } debug_printf("interactive_fd:%d\n", G_interactive_fd); if (G_interactive_fd) { - close_on_exec_on(G_interactive_fd); - if (G_saved_tty_pgrp) { /* If we were run as 'hush &', sleep until we are * in the foreground (tty pgrp == our pgrp). @@ -10796,9 +10794,6 @@ int hush_main(int argc, char **argv) G_interactive_fd = 0; } } - if (G_interactive_fd) { - close_on_exec_on(G_interactive_fd); - } install_special_sighandlers(); #else /* We have interactiveness code disabled */ -- cgit v1.2.3-55-g6feb From 14e28c18ca1a0fb87b6c73d5cb7487e80bc6713a Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 13 Jul 2024 00:59:02 +0200 Subject: hush: fix "exec 3>FILE" aborting if 3 is exactly the next free fd Signed-off-by: Denys Vlasenko --- shell/ash_test/ash-redir/redir3.right | 2 +- shell/ash_test/ash-redir/redir_exec2.right | 4 ++++ shell/ash_test/ash-redir/redir_exec2.tests | 11 +++++++++++ shell/ash_test/ash-redir/redir_to_bad_fd255.right | 2 +- shell/ash_test/ash-redir/redir_to_bad_fd3.right | 2 +- shell/hush.c | 13 +++++++++---- shell/hush_test/hush-redir/redir_exec2.right | 4 ++++ shell/hush_test/hush-redir/redir_exec2.tests | 11 +++++++++++ 8 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 shell/ash_test/ash-redir/redir_exec2.right create mode 100755 shell/ash_test/ash-redir/redir_exec2.tests create mode 100644 shell/hush_test/hush-redir/redir_exec2.right create mode 100755 shell/hush_test/hush-redir/redir_exec2.tests diff --git a/shell/ash_test/ash-redir/redir3.right b/shell/ash_test/ash-redir/redir3.right index fd641a8ea..d9cf5dac5 100644 --- a/shell/ash_test/ash-redir/redir3.right +++ b/shell/ash_test/ash-redir/redir3.right @@ -1,3 +1,3 @@ TEST -./redir3.tests: line 4: 9: Bad file descriptor +./redir3.tests: line 4: dup2(9,1): Bad file descriptor Output to fd#9: 1 diff --git a/shell/ash_test/ash-redir/redir_exec2.right b/shell/ash_test/ash-redir/redir_exec2.right new file mode 100644 index 000000000..2bdc093c9 --- /dev/null +++ b/shell/ash_test/ash-redir/redir_exec2.right @@ -0,0 +1,4 @@ +fd/5 +fd/4 +fd/3 +One:1 diff --git a/shell/ash_test/ash-redir/redir_exec2.tests b/shell/ash_test/ash-redir/redir_exec2.tests new file mode 100755 index 000000000..700a51786 --- /dev/null +++ b/shell/ash_test/ash-redir/redir_exec2.tests @@ -0,0 +1,11 @@ +cd /proc/$$ +exec 5>/dev/null +exec 4>/dev/null +exec 3>/dev/null +ls -1 fd/5 +ls -1 fd/4 +ls -1 fd/3 +exec 5>&- +test -e fd/5 && echo BUG +echo One:$? + diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd255.right b/shell/ash_test/ash-redir/redir_to_bad_fd255.right index 9c5e35b36..ca4df91ac 100644 --- a/shell/ash_test/ash-redir/redir_to_bad_fd255.right +++ b/shell/ash_test/ash-redir/redir_to_bad_fd255.right @@ -1,2 +1,2 @@ -./redir_to_bad_fd255.tests: line 2: 255: Bad file descriptor +./redir_to_bad_fd255.tests: line 2: dup2(255,1): Bad file descriptor OK diff --git a/shell/ash_test/ash-redir/redir_to_bad_fd3.right b/shell/ash_test/ash-redir/redir_to_bad_fd3.right index 895a4a0a6..5120322a5 100644 --- a/shell/ash_test/ash-redir/redir_to_bad_fd3.right +++ b/shell/ash_test/ash-redir/redir_to_bad_fd3.right @@ -1,2 +1,2 @@ -./redir_to_bad_fd3.tests: line 2: 3: Bad file descriptor +./redir_to_bad_fd3.tests: line 2: dup2(3,1): Bad file descriptor OK diff --git a/shell/hush.c b/shell/hush.c index afbc3ebec..1d5642260 100644 --- a/shell/hush.c +++ b/shell/hush.c @@ -8077,8 +8077,11 @@ static int internally_opened_fd(int fd, struct squirrel *sq) return 0; } -/* squirrel != NULL means we squirrel away copies of stdin, stdout, - * and stderr if they are redirected. */ +/* sqp != NULL means we squirrel away copies of stdin, stdout, + * and stderr if they are redirected. + * If redirection fails, return 1. This will make caller + * skip command execution and restore already created redirect fds. + */ static int setup_redirects(struct command *prog, struct squirrel **sqp) { struct redir_struct *redir; @@ -8109,7 +8112,7 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) * "cmd > rd_type].mode; p = expand_string_to_string(redir->rd_filename, @@ -8124,7 +8127,9 @@ static int setup_redirects(struct command *prog, struct squirrel **sqp) */ return 1; } - if (newfd == redir->rd_fd && sqp) { + if (newfd == redir->rd_fd && sqp + && sqp != ERR_PTR /* not a redirect in "exec" */ + ) { /* open() gave us precisely the fd we wanted. * This means that this fd was not busy * (not opened to anywhere). diff --git a/shell/hush_test/hush-redir/redir_exec2.right b/shell/hush_test/hush-redir/redir_exec2.right new file mode 100644 index 000000000..2bdc093c9 --- /dev/null +++ b/shell/hush_test/hush-redir/redir_exec2.right @@ -0,0 +1,4 @@ +fd/5 +fd/4 +fd/3 +One:1 diff --git a/shell/hush_test/hush-redir/redir_exec2.tests b/shell/hush_test/hush-redir/redir_exec2.tests new file mode 100755 index 000000000..700a51786 --- /dev/null +++ b/shell/hush_test/hush-redir/redir_exec2.tests @@ -0,0 +1,11 @@ +cd /proc/$$ +exec 5>/dev/null +exec 4>/dev/null +exec 3>/dev/null +ls -1 fd/5 +ls -1 fd/4 +ls -1 fd/3 +exec 5>&- +test -e fd/5 && echo BUG +echo One:$? + -- cgit v1.2.3-55-g6feb From 23da5c4b716b92524240c6f81c2e2474c1825cfc Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 13 Jul 2024 01:47:49 +0200 Subject: hush: do not exit interactive shell on some redirection errors $ 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 --- 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<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 -- cgit v1.2.3-55-g6feb