From 7bab9508aa8aeef98764b64953463af116a12fe4 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 19 Jan 2021 18:47:41 +0000 Subject: [PATCH] Fix crash on subshell exit if PWD is inaccessible (re: dd9bc229) This commit also further mitigates the problems with restoring an inaccessible or nonexistent PWD on exiting a virtual subshell. Harald van Dijk writes: > On a build of ksh with -fsanitize=undefined to help diagnose > problems: > > $ mkdir deleted > $ cd deleted > $ rmdir ../deleted > $ ksh -c '(cd /; (cd /)); :' > /home/harald/ksh/src/cmd/ksh93/sh/subshell.c:561:22: runtime > error: null pointer passed as argument 1, which is declared to > never be null > Segmentation fault (core dumped) > > Note that it segfaults the same with default compilation flags, > but it does not print out the useful extra message. The code > assumes that pwd is non-null and passes it to strcmp without > checking, but it will be null if the current directory cannot be > determined, for instance because it has been deleted. src/cmd/ksh93/sh/subshell.c: sh_subshell(): - Avoid the null pointer dereference reported above. src/cmd/ksh93/bltins/cd_pwd.c: b_cd(): - Fork a virtual subshell even on systems with fchdir(2) if the present working directory tests as inaccessible on invoking 'cd'; it may no longer exist and fchdir would fail to get a handle. (For the test we have to opendir(3) the full path to the PWD and not ".", as the latter may succeed even if the PWD is gone.) src/cmd/ksh93/data/builtins.c: - Update 'cd' version string. Fixes: https://github.com/ksh93/ksh/issues/153 Related: https://github.com/ksh93/ksh/issues/141 --- NEWS | 5 +++++ src/cmd/ksh93/bltins/cd_pwd.c | 13 ++++++++++--- src/cmd/ksh93/data/builtins.c | 2 +- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/subshell.c | 2 +- src/cmd/ksh93/tests/subshell.sh | 12 ++++++++++++ 6 files changed, 30 insertions(+), 6 deletions(-) diff --git a/NEWS b/NEWS index 2a2f5af01..f990d66e1 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-01-19: + +- Fixed a crash when using 'cd' in a virtual/non-forking subshell in a + situation where the current working directory cannot be determined. + 2021-01-08: - Fixed a crash on exceeding the maximum size of the $PS1 prompt. diff --git a/src/cmd/ksh93/bltins/cd_pwd.c b/src/cmd/ksh93/bltins/cd_pwd.c index 753aa1961..10b8f7174 100644 --- a/src/cmd/ksh93/bltins/cd_pwd.c +++ b/src/cmd/ksh93/bltins/cd_pwd.c @@ -37,6 +37,7 @@ #include "name.h" #include "builtins.h" #include +#include /* * Invalidate path name bindings to relative paths @@ -91,14 +92,20 @@ int b_cd(int argc, char *argv[],Shbltin_t *context) dir = nv_getval(opwdnod); if(!dir || *dir==0) errormsg(SH_DICT,ERROR_exit(1),argc==2?e_subst+4:e_direct); -#if !_lib_fchdir /* * If sh_subshell() in subshell.c cannot use fchdir(2) to restore the PWD using a saved file descriptor, * we must fork any virtual subshell now to avoid the possibility of ending up in the wrong PWD on exit. */ if(shp->subshell && !shp->subshare) - sh_subfork(); -#endif /* !lib_fchdir */ + { +#if _lib_fchdir + DIR *testdir; + if(testdir = opendir(nv_getval(pwdnod))) + closedir(testdir); + else +#endif + sh_subfork(); + } /* * Do $CDPATH processing, except if the path is absolute or the first component is '.' or '..' */ diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c index 975107ae7..f43f506d0 100644 --- a/src/cmd/ksh93/data/builtins.c +++ b/src/cmd/ksh93/data/builtins.c @@ -414,7 +414,7 @@ USAGE_LICENSE ; const char sh_optcd[] = -"[-1c?\n@(#)$Id: cd (AT&T Research) 1999-06-05 $\n]" +"[-1c?\n@(#)$Id: cd (AT&T Research/ksh93) 2021-01-19 $\n]" USAGE_LICENSE "[+NAME?cd - change working directory ]" "[+DESCRIPTION?\bcd\b changes the current working directory of the " diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index e01ec5839..9bbdc7861 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-01-08" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-01-19" /* must be in this format for $((.sh.version)) */ /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ /* Arithmetic $((.sh.version)) uses the last 10 chars, so the date must be at the end. */ diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index 283b30f8d..84bd7c0cc 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -558,7 +558,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub) #if _lib_fchdir for(xp=sp->prev; xp; xp=xp->prev) { - if(xp->pwdfd>0 && strcmp(xp->pwd,shp->pwd)==0) + if(xp->pwdfd>0 && xp->pwd && strcmp(xp->pwd,shp->pwd)==0) { sp->pwdfd = xp->pwdfd; break; diff --git a/src/cmd/ksh93/tests/subshell.sh b/src/cmd/ksh93/tests/subshell.sh index 7837a06cf..03fdb2101 100755 --- a/src/cmd/ksh93/tests/subshell.sh +++ b/src/cmd/ksh93/tests/subshell.sh @@ -893,5 +893,17 @@ got=$( { "$SHELL" "$tmp/crash_rhbz1117404.ksh"; } 2>&1) ((!(e = $?))) || err_exit 'crash while handling subshell trap' \ "(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))" +# ====== +# Segmentation fault when using cd in a subshell, when current directory cannot be determined +# https://github.com/ksh93/ksh/issues/153 +cd "$tmp" +mkdir deleted +cd deleted +tmp=$tmp "$SHELL" -c 'cd /; rmdir "$tmp/deleted"' +exp="PWD=$PWD" +got=$( { "$SHELL" -c '(cd /; (cd /)); print -r -- "PWD=$PWD"'; } 2>&1 ) +((!(e = $?))) && [[ $got == "$exp" ]] || err_exit 'failed to restore nonexistent PWD on exiting a virtual subshell' \ + "(got status $e$( ((e>128)) && print -n / && kill -l "$e"), $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125))