From 948fab26aad54896a6ae62e7720600463170d9f6 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 21 Jul 2022 00:22:30 +0200 Subject: [PATCH] Fix: running external command influences $RANDOM sequence The pseudorandom generator generates a reproducible sequnece of values after seeding it with a known value. But: $ (RANDOM=1; echo $RANDOM; echo $RANDOM) 2100 18270 $ (RANDOM=1; echo $RANDOM; ls >/dev/null; echo $RANDOM) 2100 30107 Since RANDOM was seeded with a specific value, the two command lines should output the same pair of numbers. Running 'ls' in the middle should make no difference. The cause is a nv_getval(RANDNOD) call in xec.c, case TFORK, that is run for all TFORK cases, in the parent process -- including background jobs and external commands. What should happen instead is that $RANDOM is reseeded in the child process. This bug is in every version of ksh93 since before 1995. There is also an opportunity for optimisation. As of 396b388e, the RANDOM seed may be invalidated by setting rand_last to -2, triggering a reseed the next time a $RANDOM value is obtained. This was done to optimise the virtual subshell mechanism. But that can also be used to eliminate unconditional reseeding elsewhere. So as of this commit, RANDOM is never (re)seeded until it's used. src/cmd/ksh93/include/variables.h, src/cmd/ksh93/sh/subshell.c: - Add RAND_SEED_INVALIDATED macro, a single source of truth for the value that triggers a reseeding in sh_save_rand_seed(). - Add convenient sh_invalidate_rand_seed() function macro. src/cmd/ksh93/sh/init.c, src/cmd/ksh93/sh/xec.c: - Optimisation: invalidate seed instead of reseeding directly. - sh_exec(): case TFORK: Delete the nv_getval(RANDNOD) call. Add a sh_invalidate_rand_seed() to the child part. This fixes the bug. --- NEWS | 5 +++++ src/cmd/ksh93/include/variables.h | 3 +++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/init.c | 2 +- src/cmd/ksh93/sh/subshell.c | 12 ++++-------- src/cmd/ksh93/sh/xec.c | 4 ++-- src/cmd/ksh93/tests/variables.sh | 9 +++++++++ 7 files changed, 25 insertions(+), 12 deletions(-) diff --git a/NEWS b/NEWS index 2c0fcf309..e581a8f29 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,11 @@ For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0 Any uppercase BUG_* names are modernish shell bug IDs. +2022-07-21: + +- Fixed a bug where a reproducible $RANDOM sequence (after assigning a + specific value to $RANDOM) was influenced by running any external command. + 2022-07-14: - Fixed a bug that caused a spurious "Done" message on the interactive shell diff --git a/src/cmd/ksh93/include/variables.h b/src/cmd/ksh93/include/variables.h index 6de9725b3..e570d734e 100644 --- a/src/cmd/ksh93/include/variables.h +++ b/src/cmd/ksh93/include/variables.h @@ -36,6 +36,9 @@ struct rand extern void sh_reseed_rand(struct rand *); extern void sh_save_rand_seed(struct rand *, int); +#define RAND_SEED_INVALIDATED -2 +#define sh_invalidate_rand_seed() (((struct rand*)RANDNOD->nvfun)->rand_last = RAND_SEED_INVALIDATED) + /* update ${.sh.level} and, if needed, restore the current scope */ #define update_sh_level() \ ( \ diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 20d4fae9a..abb3dbdcc 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -23,7 +23,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-beta.2" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2022-07-14" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-07-21" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index fcbec5b57..227caaa73 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -1914,7 +1914,7 @@ static Init_t *nv_init(void) nv_putval(SECONDS, (char*)&d, NV_DOUBLE); nv_stack(RANDNOD, &ip->RAND_init.hdr); nv_putval(RANDNOD, (char*)&d, NV_DOUBLE); - sh_reseed_rand((struct rand *)RANDNOD->nvfun); + sh_invalidate_rand_seed(); nv_stack(LINENO, &ip->LINENO_init); SH_MATCHNOD->nvfun = &ip->SH_MATCH_init.hdr; nv_putsub(SH_MATCHNOD,(char*)0,10); diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index ff38d12ab..9590b46b2 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -187,16 +187,12 @@ void sh_subfork(void) { /* this is the child part of the fork */ /* - * $RANDOM is only reseeded when it's used in a subshell, so if $RANDOM hasn't - * been reseeded yet set rp->rand_last to -2. This allows sh_save_rand_seed() + * In a virtual subshell, $RANDOM is not reseeded until it's used, so if that + * hasn't happened yet, invalidate the seed. This allows sh_save_rand_seed() * to reseed $RANDOM later. */ if(!sp->rand_state) - { - struct rand *rp; - rp = (struct rand*)RANDNOD->nvfun; - rp->rand_last = -2; - } + sh_invalidate_rand_seed(); subshell_data = 0; sh.subshell = 0; sh.comsub = 0; @@ -249,7 +245,7 @@ void sh_save_rand_seed(struct rand *rp, int reseed) if(reseed) sh_reseed_rand(rp); } - else if(reseed && rp->rand_last == -2) + else if(reseed && rp->rand_last == RAND_SEED_INVALIDATED) sh_reseed_rand(rp); } diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c index ee19ab87e..37f138fac 100644 --- a/src/cmd/ksh93/sh/xec.c +++ b/src/cmd/ksh93/sh/xec.c @@ -1500,7 +1500,6 @@ int sh_exec(register const Shnode_t *t, int flags) } } #endif /* SHOPT_BGX */ - nv_getval(RANDNOD); if(type&FCOOP) { pipes[2] = 0; @@ -1595,6 +1594,7 @@ int sh_exec(register const Shnode_t *t, int flags) sh.fifo_tree = NIL(Dt_t*); } #endif + sh_invalidate_rand_seed(); if(no_fork) sh_sigreset(2); sh_pushcontext(buffp,SH_JMPEXIT); @@ -1825,7 +1825,7 @@ int sh_exec(register const Shnode_t *t, int flags) sh.st.otrapcom = (char**)savsig; } /* Still act like a subshell: reseed $RANDOM and increment ${.sh.subshell} */ - sh_reseed_rand((struct rand*)RANDNOD->nvfun); + sh_invalidate_rand_seed(); sh.realsubshell++; sh_sigreset(0); sh_pushcontext(buffp,SH_JMPEXIT); diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index 1c72af658..21005b752 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -116,6 +116,15 @@ got=$(RANDOM=789; ulimit -t unlimited 2> /dev/null; echo $RANDOM) "(expected $(printf %q "$exp"), got $(printf %q "$got"))" unset N i rand1 rand2 +# Running an external command or background job should not influence the sequence +exp=$(RANDOM=1; print $RANDOM; print $RANDOM) +got=$(RANDOM=1; print $RANDOM; /dev/null/x 2>/dev/null; print $RANDOM) +[[ $got == "$exp" ]] || err_exit "External command influences reproducible $RANDOM sequence" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +got=$(RANDOM=1; print $RANDOM; :& print $RANDOM) +[[ $got == "$exp" ]] || err_exit "Background job influences reproducible $RANDOM sequence" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # SECONDS float secElapsed=0.0 secSleep=0.001 let SECONDS=$secElapsed