From 35465406cdae9cd4a15e7f6699e657b5d09bf7bd Mon Sep 17 00:00:00 2001 From: Chet Ramey Date: Fri, 2 Feb 2024 14:39:50 -0500 Subject: [PATCH] fix for cd when curent directory doesn't exist; fix wait -n in posix mode to delete any job that it returns; fix some variables where readonly can be circumvented; fix some overflows in printf --- CWRU/CWRU.chlog | 67 ++++++++++++++++++++++ builtins/cd.def | 7 ++- builtins/printf.def | 124 +++++++++++++++++++++------------------- builtins/set.def | 15 ++--- builtins/shopt.def | 15 ++--- builtins/wait.def | 55 +++++++++++++++++- general.c | 2 - jobs.c | 52 +++++++++++++++-- jobs.h | 2 + lib/readline/readline.c | 2 +- lib/readline/text.c | 9 +++ redir.c | 2 + 12 files changed, 263 insertions(+), 89 deletions(-) diff --git a/CWRU/CWRU.chlog b/CWRU/CWRU.chlog index fc38e0baf..8c5dc98ad 100644 --- a/CWRU/CWRU.chlog +++ b/CWRU/CWRU.chlog @@ -8436,3 +8436,70 @@ lib/readline/signals.c lib/readline/callback.c - _rl_callback_sigcleanup: call _rl_nsearch_sigcleanup + 1/29 + ---- +builtins/cd.def + - change_to_directory: don't try to canonicalize a NULL path that's + NULL after make_absolute. + From a report by Kerin Millar + +jobs.c + - retrieve_proc_status,delete_proc_status: external interfaces to + bgp_search and bgp_delete, respectively; these take an argument + that says whether or not to block SIGCHLD + +jobs.h + - retrieve_proc_status,delete_proc_status: extern declarations + +builtins/wait.def + - check_bgpids: new function to check whether a requested PID is in + the bgpids table (retrieve_proc_status) and optionally delete it + if it is (posixly_correct delete_proc_status) while returning its + status. If the PID isn't in the bgpids table, return -1 + - wait_builtin: if -n is supplied with pid/job arguments, use + check_bgpids to check the bgpids table for any of the requested + pids. Tagged for bash-5.3, might need another option + From a report by Steven Pelley + + 1/30 + ---- +redir.c + - redir_open: assume the AFS bug with O_CREAT and existing files in + protected directories has been fixed over the years, so take out + the workaround. + From a report by Etienne Champetier + +jobs.c + - wait_for_any_job: if the jobs list is frozen and we're running a + funsub, mark the job as notified so it gets cleaned up later + - wait_for_any_job: if we're in posix mode, we should remove the job + from the job list and not add it to the bgpids list, as posix + requires + +builtins/set.def + - set_shellopts: use ASS_FORCE in the call to bind_variable so we + don't have to mess with temporarily turning off readonly + +builtins/shopt.def + - set_bashopts: same + Report by Emanuele Torre + + 2/1 + --- +builtins/printf.def + - vblen: make it a size_t to avoid going negative on underflow + - clearerr, ferror, fflush: don't test these if vflag is set and we're + writing to a string (multiple places) + - vbprintf: pass through failure returns (< 0) from vsnprintf to the + caller(s) so we can return on errors + - printf_builtin: remove redundant test for empty or missing format + string + - PF: call builtin_error if vflag is set, sh_wrerror otherwise + (via PRETURN) + - PF: use PRETURN so we can get partial output to the variable on + error (if vflag is set) + - PRETURN: free conv_buf before attempting the write (it would get + cleaned up on the next call, but why not) + - PRETURN: clean up vbuf only if vflag is set, and clean it up on + error (it would get cleaned up on the next call, but...) + Fixes from Grisha Levit diff --git a/builtins/cd.def b/builtins/cd.def index 3e6bf4027..c8676749e 100644 --- a/builtins/cd.def +++ b/builtins/cd.def @@ -538,8 +538,11 @@ change_to_directory (char *newdir, int nolinks, int xattr) /* TDIR is either the canonicalized absolute pathname of NEWDIR (nolinks == 0) or the absolute physical pathname of NEWDIR (nolinks != 0). */ - tdir = nolinks ? sh_physpath (t, 0) - : sh_canonpath (t, PATH_CHECKDOTDOT|PATH_CHECKEXISTS); + if (t && *t) + tdir = nolinks ? sh_physpath (t, 0) + : sh_canonpath (t, PATH_CHECKDOTDOT|PATH_CHECKEXISTS); + else + tdir = NULL; ndlen = strlen (newdir); diff --git a/builtins/printf.def b/builtins/printf.def index 5e84661d0..80d536b44 100644 --- a/builtins/printf.def +++ b/builtins/printf.def @@ -1,7 +1,7 @@ This file is printf.def, from which is created printf.c. It implements the builtin "printf" in Bash. -Copyright (C) 1997-2023 Free Software Foundation, Inc. +Copyright (C) 1997-2024 Free Software Foundation, Inc. This file is part of GNU Bash, the Bourne Again SHell. @@ -105,6 +105,50 @@ $END extern int errno; #endif +/* We free the buffer used by mklong() if it's `too big'. */ +#define PRETURN(value) \ + do \ + { \ + QUIT; \ + retval = value; \ + if (conv_bufsize > 4096 ) \ + { \ + free (conv_buf); \ + conv_bufsize = 0; \ + conv_buf = 0; \ + } \ + if (vflag) \ + { \ + SHELL_VAR *v; \ + v = builtin_bind_variable (vname, vbuf, bindflags); \ + stupidly_hack_special_variables (vname); \ + if (v == 0 || readonly_p (v) || noassign_p (v)) \ + retval = EXECUTION_FAILURE; \ + if (vbsize > 4096) \ + { \ + free (vbuf); \ + vbsize = 0; \ + vbuf = 0; \ + } \ + else if (vbuf) \ + vbuf[0] = 0; \ + } \ + else \ + { \ + if (ferror (stdout) == 0) \ + fflush (stdout); \ + QUIT; \ + if (ferror (stdout)) \ + { \ + sh_wrerror (); \ + clearerr (stdout); \ + retval = EXECUTION_FAILURE; \ + } \ + } \ + return (retval); \ + } \ + while (0) + #define PC(c) \ do { \ char b[2]; \ @@ -120,7 +164,9 @@ extern int errno; #define PF(f, func) \ do { \ int nw; \ - clearerr (stdout); \ + if (vflag == 0) \ + clearerr (stdout); \ + errno = 0; \ if (have_fieldwidth && have_precision) \ nw = vflag ? vbprintf (f, fieldwidth, precision, func) : printf (f, fieldwidth, precision, func); \ else if (have_fieldwidth) \ @@ -129,56 +175,17 @@ extern int errno; nw = vflag ? vbprintf (f, precision, func) : printf (f, precision, func); \ else \ nw = vflag ? vbprintf (f, func) : printf (f, func); \ - tw += nw; \ - QUIT; \ - if (ferror (stdout)) \ + if (nw < 0 || ferror (stdout)) \ { \ - sh_wrerror (); \ - clearerr (stdout); \ - return (EXECUTION_FAILURE); \ + QUIT; \ + if (vflag) \ + builtin_error ("%s", strerror (errno)); \ + PRETURN (EXECUTION_FAILURE); \ } \ + tw += nw; \ + QUIT; \ } while (0) -/* We free the buffer used by mklong() if it's `too big'. */ -#define PRETURN(value) \ - do \ - { \ - QUIT; \ - if (vflag) \ - { \ - SHELL_VAR *v; \ - v = builtin_bind_variable (vname, vbuf, bindflags); \ - stupidly_hack_special_variables (vname); \ - if (v == 0 || readonly_p (v) || noassign_p (v)) \ - return (EXECUTION_FAILURE); \ - } \ - if (conv_bufsize > 4096 ) \ - { \ - free (conv_buf); \ - conv_bufsize = 0; \ - conv_buf = 0; \ - } \ - if (vbsize > 4096) \ - { \ - free (vbuf); \ - vbsize = 0; \ - vbuf = 0; \ - } \ - else if (vbuf) \ - vbuf[0] = 0; \ - if (ferror (stdout) == 0) \ - fflush (stdout); \ - QUIT; \ - if (ferror (stdout)) \ - { \ - sh_wrerror (); \ - clearerr (stdout); \ - return (EXECUTION_FAILURE); \ - } \ - return (value); \ - } \ - while (0) - #define SKIP1 "#'-+ 0" #define LENMODS "hjlLtz" @@ -242,7 +249,7 @@ static int vflag = 0; static int bindflags = 0; static char *vbuf, *vname; static size_t vbsize; -static int vblen; +static size_t vblen; static intmax_t tw; @@ -329,6 +336,7 @@ printf_builtin (WORD_LIST *list) return ((v == 0 || readonly_p (v) || noassign_p (v)) ? EXECUTION_FAILURE : EXECUTION_SUCCESS); } + /* If the format string is empty after preprocessing, return immediately. */ if (list->word->word == 0 || list->word->word[0] == '\0') return (EXECUTION_SUCCESS); @@ -338,10 +346,6 @@ printf_builtin (WORD_LIST *list) garglist = orig_arglist = list->next; - /* If the format string is empty after preprocessing, return immediately. */ - if (format == 0 || *format == 0) - return (EXECUTION_SUCCESS); - mb_cur_max = MB_CUR_MAX; /* Basic algorithm is to scan the format string for conversion @@ -793,7 +797,7 @@ printf_builtin (WORD_LIST *list) modstart[1] = nextch; } - if (ferror (stdout)) + if (vflag == 0 && ferror (stdout)) { /* PRETURN will print error message. */ PRETURN (EXECUTION_FAILURE); @@ -928,7 +932,7 @@ printstr (char *fmt, char *string, int len, int fieldwidth, int precision) for (; padlen < 0; padlen++) PC (' '); - return (ferror (stdout) ? -1 : 0); + return ((vflag == 0 && ferror (stdout)) ? -1 : 0); } #if defined (HANDLE_MULTIBYTE) @@ -1035,7 +1039,7 @@ printwidestr (char *fmt, wchar_t *wstring, size_t len, int fieldwidth, int preci PC (' '); free (string); - return (ferror (stdout) ? -1 : 0); + return ((vflag == 0 && ferror (stdout)) ? -1 : 0); } #endif @@ -1261,7 +1265,7 @@ vbadd (char *buf, int blen) #ifdef DEBUG if (strlen (vbuf) != vblen) - internal_error ("printf:vbadd: vblen (%d) != strlen (vbuf) (%d)", vblen, (int)strlen (vbuf)); + internal_error ("printf:vbadd: vblen (%zu) != strlen (vbuf) (%zu)", vblen, strlen (vbuf)); #endif return vbuf; @@ -1277,6 +1281,8 @@ vbprintf (const char *format, ...) va_start (args, format); blen = vsnprintf (vbuf + vblen, vbsize - vblen, format, args); va_end (args); + if (blen < 0) + return (blen); nlen = vblen + blen + 1; if (nlen >= vbsize) @@ -1286,6 +1292,8 @@ vbprintf (const char *format, ...) va_start (args, format); blen = vsnprintf (vbuf + vblen, vbsize - vblen, format, args); va_end (args); + if (blen < 0) + return (blen); } vblen += blen; @@ -1293,7 +1301,7 @@ vbprintf (const char *format, ...) #ifdef DEBUG if (strlen (vbuf) != vblen) - internal_error ("printf:vbprintf: vblen (%d) != strlen (vbuf) (%d)", vblen, (int)strlen (vbuf)); + internal_error ("printf:vbprintf: vblen (%zu) != strlen (vbuf) (%zu)", vblen, strlen (vbuf)); #endif return (blen); diff --git a/builtins/set.def b/builtins/set.def index 91cad44f2..e0024a973 100644 --- a/builtins/set.def +++ b/builtins/set.def @@ -573,17 +573,12 @@ set_shellopts (void) v = find_variable ("SHELLOPTS"); - /* Turn off the read-only attribute so we can bind the new value, and - note whether or not the variable was exported. */ - if (v) - { - VUNSETATTR (v, att_readonly); - exported = exported_p (v); - } - else - exported = 0; + /* Note whether or not the variable was exported so we can adjust after the + assignment. */ + exported = v ? exported_p (v) : 0; - v = bind_variable ("SHELLOPTS", value, 0); + /* ASS_FORCE so we don't have to temporarily turn off readonly */ + v = bind_variable ("SHELLOPTS", value, ASS_FORCE); /* Turn the read-only attribute back on, and turn off the export attribute if it was set implicitly by mark_modified_vars and SHELLOPTS was not diff --git a/builtins/shopt.def b/builtins/shopt.def index e36743c03..e6c77cc5a 100644 --- a/builtins/shopt.def +++ b/builtins/shopt.def @@ -849,17 +849,12 @@ set_bashopts (void) v = find_variable ("BASHOPTS"); - /* Turn off the read-only attribute so we can bind the new value, and - note whether or not the variable was exported. */ - if (v) - { - VUNSETATTR (v, att_readonly); - exported = exported_p (v); - } - else - exported = 0; + /* Note whether or not the variable was exported so we can adjust after the + assignment. */ + exported = v ? exported_p (v) : 0; - v = bind_variable ("BASHOPTS", value, 0); + /* ASS_FORCE so we don't have to temporarily turn off readonly */ + v = bind_variable ("BASHOPTS", value, ASS_FORCE); /* Turn the read-only attribute back on, and turn off the export attribute if it was set implicitly by mark_modified_vars and SHELLOPTS was not diff --git a/builtins/wait.def b/builtins/wait.def index 38899020f..3f8461978 100644 --- a/builtins/wait.def +++ b/builtins/wait.def @@ -92,6 +92,7 @@ int wait_intr_flag; static int set_waitlist (WORD_LIST *); static void unset_waitlist (void); +static int check_bgpids (WORD_LIST *, struct procstat *); /* Wait for the pid in LIST to stop or die. If no arguments are given, then wait for all of the active background processes of the shell and return @@ -210,6 +211,21 @@ wait_builtin (WORD_LIST *list) #if defined (JOB_CONTROL) if (nflag) { +#if 0 /* TAG:bash-5.3 stevenpelley@gmail.com 01/22/2024 */ + /* First let's see if there are any requested pids that have already + been removed from the jobs list and saved on bgpids. */ + if (list) + { + status = check_bgpids (list, &pstat); + if (status != -1) + { + if (vname) + builtin_bind_var_to_int (vname, pstat.pid, bindflags); + WAIT_RETURN (status); + } + } +#endif + if (list) { opt = set_waitlist (list); @@ -342,7 +358,7 @@ set_waitlist (WORD_LIST *list) for (l = list; l; l = l->next) { job = NO_JOB; - job = (l && valid_number (l->word->word, &pid) && pid == (pid_t) pid) + job = (l && l->word && valid_number (l->word->word, &pid) && pid == (pid_t) pid) ? get_job_by_pid ((pid_t) pid, 0, 0) : get_job_spec (l); if (job == NO_JOB || jobs == 0 || INVALID_JOB (job)) @@ -376,4 +392,41 @@ unset_waitlist (void) jobs[i]->flags &= ~J_WAITING; UNBLOCK_CHILD (oset); } + +#if 0 /* TAG:bash-5.3 */ +static int +check_bgpids (WORD_LIST *list, struct procstat *pstat) +{ + sigset_t set, oset; + pid_t pid; + intmax_t ipid; + WORD_LIST *l; + int r, s; + + r = -1; + + BLOCK_CHILD (set, oset); + for (l = list; l; l = l->next) + { + if (l && valid_number (l->word->word, &ipid) && ipid == (pid_t) ipid) + pid = ipid; + else + continue; /* skip job ids for now */ + + if ((s = retrieve_proc_status (pid, 0)) != -1) + { + pstat->pid = pid; + pstat->status = r = s; + /* If running in posix mode, `wait -n pid' deletes pid from bgpids, + just like `wait pid'. */ + if (posixly_correct) + delete_proc_status (pid, 0); + break; + } + } + UNBLOCK_CHILD (oset); + + return r; +} +#endif #endif diff --git a/general.c b/general.c index 9494604fd..636009997 100644 --- a/general.c +++ b/general.c @@ -1435,5 +1435,3 @@ default_columns (void) return (c > 0 ? c : 80); } - - diff --git a/jobs.c b/jobs.c index 3e68bf24a..f9fc82376 100644 --- a/jobs.c +++ b/jobs.c @@ -965,8 +965,10 @@ bgp_prune (void) } #endif -/* External interface to bgp_add; takes care of blocking and unblocking - SIGCHLD. Not really used. */ +/* External interface to bgp_add/bgp_search/bgp_delete; takes care of blocking + and unblocking SIGCHLD if needed. If retrieve_proc_status or delete_proc_status + is called with BLOCK non-zero, the caller must block and unblock SIGCHLD. */ + void save_proc_status (pid_t pid, int status) { @@ -977,6 +979,32 @@ save_proc_status (pid_t pid, int status) UNBLOCK_CHILD (oset); } +int +retrieve_proc_status (pid_t pid, int block) +{ + int r; + sigset_t set, oset; + + if (block) + BLOCK_CHILD (set, oset); + r = bgp_search (pid); + if (block) + UNBLOCK_CHILD (oset); + return r; +} + +void +delete_proc_status (pid_t pid, int block) +{ + sigset_t set, oset; + + if (block) + BLOCK_CHILD (set, oset); + bgp_delete (pid); + if (block) + UNBLOCK_CHILD (oset); +} + #if defined (PROCESS_SUBSTITUTION) /* Functions to add and remove PROCESS * children from the list of running asynchronous process substitutions. The list is currently a simple singly @@ -3270,8 +3298,14 @@ return_job: if (jobs_list_frozen == 0) /* must be running a funsub to get here */ { notify_of_job_status (); /* XXX */ +#if 1 /* TAG: bash-5.3 kre@munnari.oz.au 01/30/2024 */ + delete_job (i, posixly_correct ? DEL_NOBGPID : 0); +#else delete_job (i, 0); +#endif } + else + jobs[i]->flags |= J_NOTIFIED; /* clean up later */ #if defined (COPROCESS_SUPPORT) coproc_reap (); #endif @@ -4305,7 +4339,7 @@ notify_of_job_status (void) } jobs[job]->flags |= J_NOTIFIED; } - else if (job_control) /* XXX job control test added */ + else if (job_control) { if (dir == 0) dir = current_working_directory (); @@ -4315,8 +4349,16 @@ notify_of_job_status (void) _("(wd now: %s)\n"), polite_directory_format (dir)); } - /* The code above and pretty_print_job take care of setting - J_NOTIFIED as appropriate, but this is here for posterity. */ + /* This is where we set J_NOTIFIED for background jobs in + non-interactive shells without job control enabled that are + killed by SIGINT or SIGTERM (DONT_REPORT_SIGTERM) or SIGPIPE + (DONT_REPORT_SIGPIPE) as long as those signals are not + trapped, or that exit cleanly. + Interactive shells without job control enabled are handled + above. */ + /* XXX - if we want to arrange to keep these jobs in the jobs + list, instead of making them eligible to move to bgpids, + this is where to change things. */ jobs[job]->flags |= J_NOTIFIED; break; diff --git a/jobs.h b/jobs.h index c6430216b..ff6677a09 100644 --- a/jobs.h +++ b/jobs.h @@ -237,6 +237,8 @@ extern int discard_pipeline (PROCESS *); extern void append_process (char *, pid_t, int, int); extern void save_proc_status (pid_t, int); +extern int retrieve_proc_status (pid_t, int); +extern void delete_proc_status (pid_t, int); extern PROCESS *procsub_add (PROCESS *); extern PROCESS *procsub_search (pid_t); diff --git a/lib/readline/readline.c b/lib/readline/readline.c index dbe57ef6d..696eee800 100644 --- a/lib/readline/readline.c +++ b/lib/readline/readline.c @@ -1,7 +1,7 @@ /* readline.c -- a general facility for reading lines of input with emacs style editing and completion. */ -/* Copyright (C) 1987-2023 Free Software Foundation, Inc. +/* Copyright (C) 1987-2024 Free Software Foundation, Inc. This file is part of the GNU Readline Library (Readline), a library for reading lines of text with interactive input and history editing. diff --git a/lib/readline/text.c b/lib/readline/text.c index e8bf4506a..26ded8f3b 100644 --- a/lib/readline/text.c +++ b/lib/readline/text.c @@ -2093,6 +2093,9 @@ _rl_readstr_dispatch (_rl_readstr_cxt *cxt, int c) if (c < 0) c = CTRL ('C'); + /* could consider looking up the function bound to they key and dispatching + off that, but you want most characters inserted by default without having + to quote. */ switch (c) { case CTRL('W'): @@ -2168,6 +2171,12 @@ _rl_readstr_dispatch (_rl_readstr_cxt *cxt, int c) } break; +#if 0 + case CTRL('_'): + rl_do_undo (); + break; +#endif + default: #if defined (HANDLE_MULTIBYTE) if (MB_CUR_MAX > 1 && rl_byte_oriented == 0) diff --git a/redir.c b/redir.c index 6d00f242b..65ddf2016 100644 --- a/redir.c +++ b/redir.c @@ -724,6 +724,7 @@ redir_open (char *filename, int flags, int mode, enum r_instruction ri) } while (fd < 0 && errno == EINTR); +#if 0 /* TAG: bash-5.3 champetier.etienne@gmail.com 01/28/2024 */ #if defined (AFS) if ((fd < 0) && (errno == EACCES)) { @@ -731,6 +732,7 @@ redir_open (char *filename, int flags, int mode, enum r_instruction ri) errno = EACCES; /* restore errno */ } #endif /* AFS */ +#endif } return fd; -- 2.47.2