From ca5ea73d8cd4fa9f3e0c440a2c95e84c8c1169a9 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Wed, 27 Jul 2022 15:39:00 +0200 Subject: [PATCH] Fix assumption that pid_t==int32_t, other wrong typecasts For $PPID and ${.sh.pid}, ksh has simply been assuming that pid_t is a 32-bit integer. In POSIX, the size of pid_t is not specified and operating systems are free to vary it. On any system where sizeof(pid_t) != 4, these variables produce undefined results. (Note that this is not a problem with $$ as it is converted to a string value directly from pid_t in special() in macro.c.) src/cmd/ksh93/features/options: - Add feature test that produces NV_PID, a name-value attribute macro that is set to NV_INTEGER|NV_SHORT (for int16_t), NV_INTEGER (for int32_t) or NV_INTEGER|NV_LONG (for Sflong_t a.k.a. intmax_t), depending on the size of pid_t. These are the three integer size attributes supported by nv_getval(). The build will abort on any (hypothetical?) system where pid_t has a size other than these three. In that case, the test and nv_getval() will need to have a new integer size attribute added. src/cmd/ksh93/data/variables.c: - Use NV_PID instead of NV_INTEGER for $PPID and ${.sh.pid}. This fixes the issue. Other changed files: - Fix the typecasts to various fmtbase() calls; the first argument is of type intmax_t (a.k.a. Sflong_t), not long. - When outputting PIDs with sfprintf(), typecast to Sflong_t and use %lld to account for possible systems with very large PIDs. (Though %lld is not in C89/C90, Sfio supports it anyway.) --- src/cmd/ksh93/data/variables.c | 4 ++-- src/cmd/ksh93/edit/completion.c | 2 +- src/cmd/ksh93/edit/emacs.c | 2 +- src/cmd/ksh93/edit/history.c | 2 +- src/cmd/ksh93/features/options | 36 +++++++++++++++++++++++++++++++++ src/cmd/ksh93/sh/args.c | 2 +- src/cmd/ksh93/sh/array.c | 2 +- src/cmd/ksh93/sh/init.c | 6 +++--- src/cmd/ksh93/sh/io.c | 5 ++--- src/cmd/ksh93/sh/jobs.c | 18 ++++++++--------- src/cmd/ksh93/sh/lex.c | 2 +- src/cmd/ksh93/sh/macro.c | 2 +- src/cmd/ksh93/sh/main.c | 2 +- src/cmd/ksh93/sh/path.c | 4 ++-- 14 files changed, 62 insertions(+), 27 deletions(-) diff --git a/src/cmd/ksh93/data/variables.c b/src/cmd/ksh93/data/variables.c index 17acfc83b..51b50dc86 100644 --- a/src/cmd/ksh93/data/variables.c +++ b/src/cmd/ksh93/data/variables.c @@ -64,7 +64,7 @@ const struct shtable2 shtab_variables[] = "VISUAL", 0, (char*)0, "COLUMNS", 0, (char*)0, "LINES", 0, (char*)0, - "PPID", NV_NOFREE|NV_INTEGER, (char*)0, + "PPID", NV_NOFREE|NV_PID, (char*)0, "_", NV_EXPORT, (char*)0, "TMOUT", NV_NOFREE|NV_INTEGER, (char*)0, "SECONDS", NV_NOFREE|NV_INTEGER|NV_DOUBLE, (char*)0, @@ -103,7 +103,7 @@ const struct shtable2 shtab_variables[] = ".sh.stats", 0, (char*)0, ".sh.math", 0, (char*)0, ".sh.pool", 0, (char*)0, - ".sh.pid", NV_INTEGER|NV_NOFREE, (char*)0, + ".sh.pid", NV_PID|NV_NOFREE, (char*)0, ".sh.tilde", 0, (char*)0, "SHLVL", NV_INTEGER|NV_NOFREE|NV_EXPORT, (char*)0, "", 0, (char*)0 diff --git a/src/cmd/ksh93/edit/completion.c b/src/cmd/ksh93/edit/completion.c index 661012cda..8a2fe6ba1 100644 --- a/src/cmd/ksh93/edit/completion.c +++ b/src/cmd/ksh93/edit/completion.c @@ -627,7 +627,7 @@ int ed_fulledit(Edit_t *ep) hist_flush(sh.hist_ptr); } cp = strcopy((char*)ep->e_inbuf,e_runvi); - cp = strcopy(cp, fmtbase((long)ep->e_hline,10,0)); + cp = strcopy(cp, fmtbase((intmax_t)ep->e_hline,10,0)); #if SHOPT_VSH ep->e_eol = ((unsigned char*)cp - (unsigned char*)ep->e_inbuf)-(sh_isoption(SH_VI)!=0); #else diff --git a/src/cmd/ksh93/edit/emacs.c b/src/cmd/ksh93/edit/emacs.c index 0e372ce59..093e6ce8a 100644 --- a/src/cmd/ksh93/edit/emacs.c +++ b/src/cmd/ksh93/edit/emacs.c @@ -1288,7 +1288,7 @@ static void xcommands(register Emacs_t *ep,int count) } return; -# define itos(i) fmtbase((long)(i),0,0)/* want signed conversion */ +# define itos(i) fmtbase((intmax_t)(i),0,0) /* want signed conversion */ case cntl('H'): /* ^X^H show history info */ { diff --git a/src/cmd/ksh93/edit/history.c b/src/cmd/ksh93/edit/history.c index 36c710708..f81ac2a66 100644 --- a/src/cmd/ksh93/edit/history.c +++ b/src/cmd/ksh93/edit/history.c @@ -316,7 +316,7 @@ retry: if(hist_clean(fd) && hist_start>1 && hsize > HIST_MAX) { #ifdef DEBUG - sfprintf(sfstderr,"%d: hist_trim hsize=%d\n",sh.current_pid,hsize); + sfprintf(sfstderr,"%lld: hist_trim hsize=%d\n",(Sflong_t)sh.current_pid,hsize); sfsync(sfstderr); #endif /* DEBUG */ hp = hist_trim(hp,(int)hp->histind-maxlines); diff --git a/src/cmd/ksh93/features/options b/src/cmd/ksh93/features/options index c90ab74c5..78458d8d2 100644 --- a/src/cmd/ksh93/features/options +++ b/src/cmd/ksh93/features/options @@ -67,3 +67,39 @@ tst note{ can we probe file system case insensitivity }end output{ return !r; } }end + +tst note{ determining size of PID variables }end output{ + #include + #include + int main(void) + { + const int s = sizeof(pid_t); + char *a; + Sfio_t *b; + /* + * For $$, $PPID, ${.sh.pid} and ${.sh.ppid} to work correctly, the size + * of pid_t must be one of the integer sizes supported by nv_getval(). + * If there is ever a system where this test returns 1, then both this + * test and nv_getval() must learn a new integer size attribute. + */ + if(s==sizeof(int16_t)) /* union Value: sp */ + a="NV_INTEGER|NV_SHORT"; + else if(s==sizeof(int32_t)) /* union Value: lp */ + a="NV_INTEGER"; + else if(s==sizeof(Sflong_t)) /* union Value: llp */ + a="NV_INTEGER|NV_LONG"; + else + return 1; + /* write NV_PID attribute definition */ + sfprintf(sfstdout,"#define NV_PID\t(%s)\t/* integer attribute(s) corresponding to sizeof(pid_t)==%d */\n",a,s); + /* show nice test result output on FD 9 */ + b = sfstropen(); + sfprintf(b," %s (%d bytes) ... ",a,s); + a = sfstruse(b); + write(9,a,strlen(a)); + return 0; + } +}end fail{ + echo "$0: cannot determine size of PID variables" >&2 + exit 1 +}end diff --git a/src/cmd/ksh93/sh/args.c b/src/cmd/ksh93/sh/args.c index 18c76efe6..582eb1a9c 100644 --- a/src/cmd/ksh93/sh/args.c +++ b/src/cmd/ksh93/sh/args.c @@ -744,7 +744,7 @@ struct argnod *sh_argprocsub(struct argnod *argp) chmod(sh.fifo,S_IRUSR|S_IWUSR); /* mkfifo + chmod works regardless of umask */ sfputr(sh.stk,sh.fifo,0); #endif /* SHOPT_DEVFD */ - sfputr(sh.stk,fmtbase((long)pv[fd],10,0),0); + sfputr(sh.stk,fmtbase((intmax_t)pv[fd],10,0),0); ap = (struct argnod*)stkfreeze(sh.stk,0); sh.inpipe = sh.outpipe = 0; /* turn off job control */ diff --git a/src/cmd/ksh93/sh/array.c b/src/cmd/ksh93/sh/array.c index 2f73d1108..24be646af 100644 --- a/src/cmd/ksh93/sh/array.c +++ b/src/cmd/ksh93/sh/array.c @@ -811,7 +811,7 @@ static struct index_array *array_grow(Namval_t *np, register struct index_array register int newsize = arsize(arp,maxi+1); if (maxi >= ARRAY_MAX) { - errormsg(SH_DICT,ERROR_exit(1),e_subscript, fmtbase((long)maxi,10,0)); + errormsg(SH_DICT,ERROR_exit(1),e_subscript, fmtbase((intmax_t)maxi,10,0)); UNREACHABLE(); } i = (newsize-1)*sizeof(union Value)+newsize; diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index 0314bae08..c132ff187 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -717,7 +717,7 @@ static Sfdouble_t nget_rand(register Namval_t* np, Namfun_t *fp) static char* get_rand(register Namval_t* np, Namfun_t *fp) { - register long n = nget_rand(np,fp); + intmax_t n = (intmax_t)nget_rand(np,fp); return(fmtbase(n, 10, 0)); } @@ -728,7 +728,7 @@ void sh_reseed_rand(struct rand *rp) static unsigned int seq; timeofday(&tp); time = (unsigned int)remainder(dtime(&tp) * 10000.0, (double)UINT_MAX); - srand(rp->rand_seed = sh.current_pid ^ time ^ ++seq); + srand(rp->rand_seed = (unsigned int)sh.current_pid ^ time ^ ++seq); rp->rand_last = -1; } @@ -767,7 +767,7 @@ static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp) static char* get_lineno(register Namval_t* np, Namfun_t *fp) { - long n = (long)nget_lineno(np,fp); + intmax_t n = (intmax_t)nget_lineno(np,fp); return(fmtbase(n, 10, 0)); } diff --git a/src/cmd/ksh93/sh/io.c b/src/cmd/ksh93/sh/io.c index 5bc48e9dc..3e8fecd05 100644 --- a/src/cmd/ksh93/sh/io.c +++ b/src/cmd/ksh93/sh/io.c @@ -1092,8 +1092,7 @@ static char *io_usename(char *name, int *perm, int fno, int mode) ep = sp; stakseek(0); } - stakputc('.'); - sfprintf(stkstd,"%<#d_%d{;.tmp",sh.current_pid,fno); + sfprintf(stkstd, ".<#%lld_%d{;.tmp", (Sflong_t)sh.current_pid, fno); tname = stakfreeze(1); switch(mode) { @@ -2227,7 +2226,7 @@ static void sftrack(Sfio_t* sp, int flag, void* data) #ifdef DEBUG if(flag==SF_READ || flag==SF_WRITE) { - char *z = fmtbase((long)sh.current_pid,0,0); + char *z = fmtbase((intmax_t)sh.current_pid,0,0); write(ERRIO,z,strlen(z)); write(ERRIO,": ",2); write(ERRIO,"attempt to ",11); diff --git a/src/cmd/ksh93/sh/jobs.c b/src/cmd/ksh93/sh/jobs.c index 02473fc64..3227bc06f 100644 --- a/src/cmd/ksh93/sh/jobs.c +++ b/src/cmd/ksh93/sh/jobs.c @@ -299,7 +299,7 @@ int job_reap(register int sig) abort(); } #ifdef DEBUG - if(sfprintf(sfstderr,"ksh: job line %4d: reap PID=%d critical=%d signal=%d\n",__LINE__,sh.current_pid,job.in_critical,sig) <=0) + if(sfprintf(sfstderr,"ksh: job line %4d: reap PID=%lld critical=%d signal=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,sig) <=0) write(2,"waitsafe\n",9); sfsync(sfstderr); #endif /* DEBUG */ @@ -352,7 +352,7 @@ int job_reap(register int sig) if(!(pw=job_bypid(pid))) { #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: reap PID=%d critical=%d unknown job PID=%d pw=%x\n",__LINE__,sh.current_pid,job.in_critical,pid,pw); + sfprintf(sfstderr,"ksh: job line %4d: reap PID=%lld critical=%d unknown job PID=%d pw=%x\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pid,pw); #endif /* DEBUG */ if (WIFCONTINUED(wstat) && wcontinued) continue; @@ -460,7 +460,7 @@ int job_reap(register int sig) jp->exitval |= SH_EXITSIG; } #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: reap PID=%d critical=%d job %d with PID %d flags=%o complete with status=%x exit=%d\n",__LINE__,sh.current_pid,job.in_critical,pw->p_job,pid,pw->p_flag,wstat,pw->p_exit); + sfprintf(sfstderr,"ksh: job line %4d: reap PID=%lld critical=%d job %d with PID %d flags=%o complete with status=%x exit=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pw->p_job,pid,pw->p_flag,wstat,pw->p_exit); sfsync(sfstderr); #endif /* DEBUG */ /* only top-level process in job should have notify set */ @@ -1300,7 +1300,7 @@ int job_post(pid_t pid, pid_t join) pw->p_fgrp = 0; pw->p_pgrp = pw->p_fgrp; #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: post PID=%d critical=%d job=%d PID=%d PGID=%d savesig=%d join=%d\n",__LINE__,sh.current_pid,job.in_critical,pw->p_job, + sfprintf(sfstderr,"ksh: job line %4d: post PID=%lld critical=%d job=%d PID=%d PGID=%d savesig=%d join=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pw->p_job, pw->p_pid,pw->p_pgrp,job.savesig,join); sfsync(sfstderr); #endif /* DEBUG */ @@ -1448,9 +1448,9 @@ int job_wait(register pid_t pid) } pwfg = pw; #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: wait PID=%d critical=%d job=%d PID=%d\n",__LINE__,sh.current_pid,job.in_critical,jobid,pid); + sfprintf(sfstderr,"ksh: job line %4d: wait PID=%lld critical=%d job=%d PID=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,jobid,pid); if(pw) - sfprintf(sfstderr,"ksh: job line %4d: wait PID=%d critical=%d flags=%o\n",__LINE__,sh.current_pid,job.in_critical,pw->p_flag); + sfprintf(sfstderr,"ksh: job line %4d: wait PID=%lld critical=%d flags=%o\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pw->p_flag); #endif /* DEBUG */ errno = 0; if(sh.coutpipe>=0 && lastpid && sh.cpid==lastpid) @@ -1698,7 +1698,7 @@ static struct process *job_unpost(register struct process *pwtop,int notify) register struct process *pw; /* make sure all processes are done */ #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: drop PID=%d critical=%d PID=%d env=%u\n",__LINE__,sh.current_pid,job.in_critical,pwtop->p_pid,pwtop->p_env); + sfprintf(sfstderr,"ksh: job line %4d: drop PID=%lld critical=%d PID=%d env=%u\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pwtop->p_pid,pwtop->p_env); sfsync(sfstderr); #endif /* DEBUG */ pwtop = pw = job_byjid((int)pwtop->p_job); @@ -1739,7 +1739,7 @@ static struct process *job_unpost(register struct process *pwtop,int notify) } pwtop->p_pid = 0; #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: free PID=%d critical=%d job=%d\n",__LINE__,sh.current_pid,job.in_critical,pwtop->p_job); + sfprintf(sfstderr,"ksh: job line %4d: free PID=%lld critical=%d job=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,pwtop->p_job); sfsync(sfstderr); #endif /* DEBUG */ job_free((int)pwtop->p_job); @@ -1935,7 +1935,7 @@ void job_subrestore(void* ptr) void job_fork(pid_t parent) { #ifdef DEBUG - sfprintf(sfstderr,"ksh: job line %4d: fork PID=%d critical=%d parent=%d\n",__LINE__,sh.current_pid,job.in_critical,parent); + sfprintf(sfstderr,"ksh: job line %4d: fork PID=%lld critical=%d parent=%d\n",__LINE__,(Sflong_t)sh.current_pid,job.in_critical,parent); #endif /* DEBUG */ switch (parent) { diff --git a/src/cmd/ksh93/sh/lex.c b/src/cmd/ksh93/sh/lex.c index a4efe8427..370c19d44 100644 --- a/src/cmd/ksh93/sh/lex.c +++ b/src/cmd/ksh93/sh/lex.c @@ -233,7 +233,7 @@ int sh_lex(Lex_t *lp) if(flag&ARG_QUOTED) quoted = "quoted:"; } - sfprintf(sfstderr,"%d: line %d: %o:%s%s%s%s %s\n",sh.current_pid,sh.inlineno,tok,quoted, + sfprintf(sfstderr,"%lld: line %d: %o:%s%s%s%s %s\n",(Sflong_t)sh.current_pid,sh.inlineno,tok,quoted, macro, split, expand, fmttoken(lp,tok)); return(tok); } diff --git a/src/cmd/ksh93/sh/macro.c b/src/cmd/ksh93/sh/macro.c index 6a2d7efa5..4017f8116 100644 --- a/src/cmd/ksh93/sh/macro.c +++ b/src/cmd/ksh93/sh/macro.c @@ -80,7 +80,7 @@ typedef struct _mac_ #define isescchar(s) ((s)>S_QUOTE) #define isqescchar(s) ((s)>=S_QUOTE) #define isbracechar(c) ((c)==RBRACE || (_c_=sh_lexstates[ST_BRACE][c])==S_MOD1 ||_c_==S_MOD2) -#define ltos(x) fmtbase((long)(x),0,0) +#define ltos(x) fmtbase((intmax_t)(x),0,0) /* type of macro expansions */ #define M_BRACE 1 /* ${var} */ diff --git a/src/cmd/ksh93/sh/main.c b/src/cmd/ksh93/sh/main.c index 8ad8897cb..dd6a21230 100644 --- a/src/cmd/ksh93/sh/main.c +++ b/src/cmd/ksh93/sh/main.c @@ -528,7 +528,7 @@ static void exfile(register Sfio_t *iop,register int fno) { buff.mode = SH_JMPERREXIT; #ifdef DEBUG - errormsg(SH_DICT,ERROR_warn(0),"%d: mode changed to SH_JMPERREXIT",sh.current_pid); + errormsg(SH_DICT,ERROR_warn(0),"%lld: mode changed to SH_JMPERREXIT",(Sflong_t)sh.current_pid); #endif } } diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 4eeee6a35..04bafea8e 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -1073,7 +1073,7 @@ pid_t path_spawn(const char *opath,register char **argv, char **envp, Pathcomp_t #if _lib_readlink /* save original pathname */ stakseek(PATH_OFFSET); - pidsize = sfprintf(stkstd,"*%d*",spawn?sh.current_pid:getppid()); + pidsize = sfprintf(stkstd, "*%lld*", (Sflong_t)(spawn ? sh.current_pid : sh.current_ppid)); stakputs(opath); opath = stakfreeze(1)+PATH_OFFSET+pidsize; /* only use tracked alias if we're not searching default path */ @@ -1336,7 +1336,7 @@ static noreturn void exscript(register char *path,register char *argv[],char **e } if((euserid=geteuid()) != sh.userid) { - strncpy(name+9,fmtbase((long)sh.current_pid,10,0),sizeof(name)-10); + strncpy(name+9,fmtbase((intmax_t)sh.current_pid,10,0),sizeof(name)-10); /* create an SUID open file with owner equal to effective UID */ if((n=open(name,O_CREAT|O_TRUNC|O_WRONLY,S_ISUID|S_IXUSR)) < 0) goto fail;