From cefe087d238c5ceda568513614d7dacc4515d201 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Fri, 25 Sep 2020 08:19:43 +0200 Subject: [PATCH] Fix argv rewrite on invoking hashbangless script (rhbz#1047506) The fixargs() function is invoked when ksh needs to run a script without a #!/hashbang/path. Instead of letting the kernel invoke a shell, ksh exfile()s the script itself from sh_main(). In the forked child, it calls fixargs() to set the argument list in the environment to the args of the new script, so that 'ps' and /proc/PID/cmdline show the expected output. But fixargs() is broken because, on systems other than HP-UX (on which ksh uses pstat(2)), ksh simply inserts a terminating zero. The arguments list is not a zero-terminated C string. Unix systems expect the entire arguments buffer to be zeroed out, otherwise 'ps' and /proc/*/cmdline will have fragments of previous command lines in the output. The Red Hat patch for this bug is: https://src.fedoraproject.org/rpms/ksh/blob/642af4d6/f/ksh-20120801-argvfix.patch However, that fix is incomplete because 'command_len' was also hardcoded to be limited to 64 characters (!), which still gave invalid 'ps' output if the erased command line was longer. src/cmd/ksh93/sh/main.c: fixargs(): - Remove CMD_LENGTH macro which was defined as 64. - Remove code that limited the erasure of the arguments buffer to CMD_LENGTH characters. That code also had quite a dodgy strdup() call -- it copies arguments to the heap, but they are never freed (or even used), so it's a memory leak. Also, none of this is ever done if the length is calculated using pstat(2) on HP-UX, which is a clear indication that it's unnecessary. (I think this code block must have been some experiment they forgot to remove. One reason why I think so is that a 64 byte arguments limit never made sense, even in the 1980s when they wrote ksh on 80-column CRT displays. Another indication of this is that fixing it didn't require adding anything; the code to do the right thing was already there, it was just being overridden.) - Zero out the full arguments length as in the Red Hat patch. src/cmd/ksh93/tests/basic.sh: - Add test. It's sort of involved because 'ps' is one of the least portable commands in practice, in spite of standardisation. --- NEWS | 7 +++++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/main.c | 26 ++++++++++---------------- src/cmd/ksh93/tests/basic.sh | 24 ++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 17 deletions(-) diff --git a/NEWS b/NEWS index 46ec224cc315..e6d915d4f10a 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,13 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2020-09-25: + +- When ksh invoked a shell script that does not have a leading + #!/hashbang/path, 'ps' and /proc//cmdline showed corrupted output if + the new script's command line was shorter than that of the invoking script. + This has been fixed by wiping the arguments buffer correctly. + 2020-09-24: - An omission made it impossible to turn off brace expansion within command diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 1c08d8894ce7..adb6e8ad908a 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -17,4 +17,4 @@ * David Korn * * * ***********************************************************************/ -#define SH_RELEASE "93u+m 2020-09-24" +#define SH_RELEASE "93u+m 2020-09-25" diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index 54e54d2b2f1f..47bd007cc7a1 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/src/cmd/ksh93/sh/main.c @@ -49,8 +49,6 @@ # include #endif /* _hdr_nc */ -#define CMD_LENGTH 64 - /* These routines are referenced by this module */ static void exfile(Shell_t*, Sfio_t*,int); static void chkmail(Shell_t *shp, char*); @@ -340,6 +338,7 @@ int sh_main(int ac, char *av[], Shinit_f userinit) } else { + /* beenhere > 0: We're in a forked child, about to execute a script without a hashbang path. */ fdin = shp->infd; fixargs(shp->st.dolv,1); } @@ -697,6 +696,13 @@ static void chkmail(Shell_t *shp, char *files) #if defined(_lib_fork) && !defined(_NEXT_SOURCE) /* * fix up command line for ps command + * + * This function is invoked when ksh needs to run a script without a + * #!/hashbang/path. Instead of letting the kernel invoke a shell, ksh + * exfile()s the script itself from sh_main(). In the forked child, it calls + * fixargs() to set the argument list in the environment to the args of the + * new script, so that 'ps' and /proc/PID/cmdline show the expected output. + * * mode is 0 for initialization */ static void fixargs(char **argv, int mode) @@ -727,19 +733,6 @@ static void fixargs(char **argv, int mode) buff = argv[0]; while(cp = *argv++) command_len += strlen(cp)+1; - if(environ && *environ==buff+command_len) - { - for(argv=environ; cp = *argv; cp++) - { - if(command_len > CMD_LENGTH) - { - command_len = CMD_LENGTH; - break; - } - *argv++ = strdup(cp); - command_len += strlen(cp)+1; - } - } command_len -= 1; return; } @@ -754,7 +747,8 @@ static void fixargs(char **argv, int mode) offset += size; buff[offset++] = ' '; } - buff[offset-1] = 0; + offset--; + memset(&buff[offset], 0, command_len - offset); # ifdef PSTAT un.pst_command = stakptr(0); pstat(PSTAT_SETCMD,un,0,0,0); diff --git a/src/cmd/ksh93/tests/basic.sh b/src/cmd/ksh93/tests/basic.sh index 035206ab02d0..4074542bbd02 100755 --- a/src/cmd/ksh93/tests/basic.sh +++ b/src/cmd/ksh93/tests/basic.sh @@ -692,5 +692,29 @@ actual=$(exptest foo) [[ $actual == "$expect" ]] || err_exit 'Corruption of multibyte char following expansion of single-char name' \ "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" +# ====== +# ksh didn't rewrite argv correctly (rhbz#1047506) +# When running a script without a #! hashbang path, ksh attempts to replace argv with the arguments +# of the script. However, fixargs() didn't wipe out the rest of previous arguments after the last +# \0. This caused an erroneous record in /proc//cmdline and the output of the ps command. +if actual=$(UNIX95=1 ps -o args= -p "$$" 2>&1) # UNIX95=1 makes this work on HP-UX + # Some 'ps' implementations add leading and/or trailing whitespace. Remove. + while [[ $actual == [[:space:]]* ]]; do actual=${actual#?}; done + while [[ $actual == *[[:space:]] ]]; do actual=${actual%?}; done + [[ $actual == "$SHELL $0" ]] # this is how shtests invokes this script +then expect='./atest 1 2' + echo 'sleep 10; exit 0' >atest + chmod 755 atest + ./atest 1 2 & + actual=$(UNIX95=1 ps -o args= -p "$!") + kill "$!" + while [[ $actual == [[:space:]]* ]]; do actual=${actual#?}; done + while [[ $actual == *[[:space:]] ]]; do actual=${actual%?}; done + [[ $actual == "$expect" ]] || err_exit "ksh didn't rewrite argv correctly" \ + "(expected $(printf %q "$expect"), got $(printf %q "$actual"))" +else err_exit "warning: skipping argv rewrite test due to noncompliant 'ps' utility (got $(printf %q "$actual"))" + let Errors-- +fi + # ====== exit $((Errors<125?Errors:125))