Module Name: src Committed By: kre Date: Fri Jun 22 11:04:55 UTC 2018
Modified Files: src/bin/sh: eval.c exec.c exec.h mknodes.sh nodes.c.pat Log Message: Deal with ref after free found by ASAN when a function redefines itself, or some other function which is still active. This was a long known bug (fixed ages ago in the FreeBSD sh) which hadn't been fixed as in practice, the situation that causes the problem simply doesn't arise .. ASAN found it in the sh dotcmd tests which do have this odd "feature" in the way they are written (but where it never caused a problem, as the tests are so simple that no mem is ever allocated between when the old version of the function was deleted, and when it finished executing, so its code all remained intact, despite having been freed.) The fix is taken from the FreeBSD sh. XXX -- pullup-8 (after a while to ensure no other problems arise). To generate a diff of this commit: cvs rdiff -u -r1.154 -r1.155 src/bin/sh/eval.c cvs rdiff -u -r1.51 -r1.52 src/bin/sh/exec.c cvs rdiff -u -r1.26 -r1.27 src/bin/sh/exec.h cvs rdiff -u -r1.2 -r1.3 src/bin/sh/mknodes.sh cvs rdiff -u -r1.13 -r1.14 src/bin/sh/nodes.c.pat Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/bin/sh/eval.c diff -u src/bin/sh/eval.c:1.154 src/bin/sh/eval.c:1.155 --- src/bin/sh/eval.c:1.154 Sun Jun 17 17:19:06 2018 +++ src/bin/sh/eval.c Fri Jun 22 11:04:55 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: eval.c,v 1.154 2018/06/17 17:19:06 kre Exp $ */ +/* $NetBSD: eval.c,v 1.155 2018/06/22 11:04:55 kre Exp $ */ /*- * Copyright (c) 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)eval.c 8.9 (Berkeley) 6/8/95"; #else -__RCSID("$NetBSD: eval.c,v 1.154 2018/06/17 17:19:06 kre Exp $"); +__RCSID("$NetBSD: eval.c,v 1.155 2018/06/22 11:04:55 kre Exp $"); #endif #endif /* not lint */ @@ -1069,6 +1069,7 @@ evalcommand(union node *cmd, int flgs, s INTOFF; savelocalvars = localvars; localvars = NULL; + reffunc(cmdentry.u.func); INTON; if (setjmp(jmploc.loc)) { if (exception == EXSHELLPROC) { @@ -1078,6 +1079,7 @@ evalcommand(union node *cmd, int flgs, s freeparam(&shellparam); shellparam = saveparam; } + unreffunc(cmdentry.u.func); poplocalvars(); localvars = savelocalvars; funclinebase = savefuncline; @@ -1096,8 +1098,8 @@ evalcommand(union node *cmd, int flgs, s VTRACE(DBG_EVAL, ("function: node: %d '%s' # %d%s; funclinebase=%d\n", - cmdentry.u.func->type, - NODETYPENAME(cmdentry.u.func->type), + getfuncnode(cmdentry.u.func)->type, + NODETYPENAME(getfuncnode(cmdentry.u.func)->type), cmdentry.lineno, cmdentry.lno_frel?" (=1)":"", funclinebase)); } @@ -1105,9 +1107,10 @@ evalcommand(union node *cmd, int flgs, s /* stop shell blowing its stack */ if (++funcnest > 1000) error("too many nested function calls"); - evaltree(cmdentry.u.func, flags & EV_TESTED); + evaltree(getfuncnode(cmdentry.u.func), flags & EV_TESTED); funcnest--; INTOFF; + unreffunc(cmdentry.u.func); poplocalvars(); localvars = savelocalvars; funclinebase = savefuncline; Index: src/bin/sh/exec.c diff -u src/bin/sh/exec.c:1.51 src/bin/sh/exec.c:1.52 --- src/bin/sh/exec.c:1.51 Wed Jul 5 19:58:10 2017 +++ src/bin/sh/exec.c Fri Jun 22 11:04:55 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: exec.c,v 1.51 2017/07/05 19:58:10 kre Exp $ */ +/* $NetBSD: exec.c,v 1.52 2018/06/22 11:04:55 kre Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -37,7 +37,7 @@ #if 0 static char sccsid[] = "@(#)exec.c 8.4 (Berkeley) 6/8/95"; #else -__RCSID("$NetBSD: exec.c,v 1.51 2017/07/05 19:58:10 kre Exp $"); +__RCSID("$NetBSD: exec.c,v 1.52 2018/06/22 11:04:55 kre Exp $"); #endif #endif /* not lint */ @@ -481,8 +481,9 @@ printentry(struct tblentry *cmdp, int ve out1fmt("%s", cmdp->cmdname); if (verbose) { struct procstat ps; + INTOFF; - commandtext(&ps, cmdp->param.func); + commandtext(&ps, getfuncnode(cmdp->param.func)); INTON; out1str("() { "); out1str(ps.cmd); @@ -989,9 +990,8 @@ addcmdentry(char *name, struct cmdentry INTOFF; cmdp = cmdlookup(name, 1); if (cmdp->cmdtype != CMDSPLBLTIN) { - if (cmdp->cmdtype == CMDFUNCTION) { - freefunc(cmdp->param.func); - } + if (cmdp->cmdtype == CMDFUNCTION) + unreffunc(cmdp->param.func); cmdp->cmdtype = entry->cmdtype; cmdp->lineno = entry->lineno; cmdp->fn_ln1 = entry->lno_frel; @@ -1031,7 +1031,7 @@ unsetfunc(char *name) if ((cmdp = cmdlookup(name, 0)) != NULL && cmdp->cmdtype == CMDFUNCTION) { - freefunc(cmdp->param.func); + unreffunc(cmdp->param.func); delete_cmd_entry(); } return 0; Index: src/bin/sh/exec.h diff -u src/bin/sh/exec.h:1.26 src/bin/sh/exec.h:1.27 --- src/bin/sh/exec.h:1.26 Wed Jun 7 05:08:32 2017 +++ src/bin/sh/exec.h Fri Jun 22 11:04:55 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: exec.h,v 1.26 2017/06/07 05:08:32 kre Exp $ */ +/* $NetBSD: exec.h,v 1.27 2018/06/22 11:04:55 kre Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -49,7 +49,7 @@ struct cmdentry { union param { int index; int (*bltin)(int, char**); - union node *func; + struct funcdef *func; } u; }; @@ -72,6 +72,7 @@ void hashcd(void); void changepath(const char *); void deletefuncs(void); void getcmdentry(char *, struct cmdentry *); +union node; void defun(char *, union node *, int); int unsetfunc(char *); void hash_special_builtins(void); Index: src/bin/sh/mknodes.sh diff -u src/bin/sh/mknodes.sh:1.2 src/bin/sh/mknodes.sh:1.3 --- src/bin/sh/mknodes.sh:1.2 Tue Apr 29 06:53:00 2008 +++ src/bin/sh/mknodes.sh Fri Jun 22 11:04:55 2018 @@ -1,5 +1,5 @@ #! /bin/sh -# $NetBSD: mknodes.sh,v 1.2 2008/04/29 06:53:00 martin Exp $ +# $NetBSD: mknodes.sh,v 1.3 2018/06/22 11:04:55 kre Exp $ # Copyright (c) 2003 The NetBSD Foundation, Inc. # All rights reserved. @@ -111,8 +111,12 @@ echo " union node *n;" echo "};" echo echo -echo "union node *copyfunc(union node *);" -echo "void freefunc(union node *);" +echo 'struct funcdef;' +echo 'struct funcdef *copyfunc(union node *);' +echo 'union node *getfuncnode(struct funcdef *);' +echo 'void reffunc(struct funcdef *);' +echo 'void unreffunc(struct funcdef *);' +echo 'void freefunc(struct funcdef *);' mv $objdir/nodes.h.tmp $objdir/nodes.h || exit 1 @@ -140,7 +144,7 @@ while IFS=; read -r line; do '%CALCSIZE' ) echo " if (n == NULL)" echo " return;" - echo " funcblocksize += nodesize[n->type];" + echo " res->bsize += nodesize[n->type];" echo " switch (n->type) {" IFS=' ' for struct in $struct_list; do @@ -157,11 +161,11 @@ while IFS=; read -r line; do IFS=' ' set -- $line name=$1 - cl=")" + cl=", res)" case $2 in nodeptr ) fn=calcsize;; nodelist ) fn=sizenodelist;; - string ) fn="funcstringsize += strlen" + string ) fn="res->ssize += strlen" cl=") + 1";; * ) continue;; esac @@ -174,8 +178,8 @@ while IFS=; read -r line; do '%COPY' ) echo " if (n == NULL)" echo " return NULL;" - echo " new = funcblock;" - echo " funcblock = (char *) funcblock + nodesize[n->type];" + echo " new = st->block;" + echo " st->block = (char *) st->block + nodesize[n->type];" echo " switch (n->type) {" IFS=' ' for struct in $struct_list; do @@ -200,7 +204,7 @@ while IFS=; read -r line; do * ) continue;; esac f="$struct.$name" - echo " new->$f = ${fn}n->$f${fn:+)};" + echo " new->$f = ${fn}n->$f${fn:+, st)};" done echo " break;" done Index: src/bin/sh/nodes.c.pat diff -u src/bin/sh/nodes.c.pat:1.13 src/bin/sh/nodes.c.pat:1.14 --- src/bin/sh/nodes.c.pat:1.13 Tue Mar 20 18:42:29 2012 +++ src/bin/sh/nodes.c.pat Fri Jun 22 11:04:55 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: nodes.c.pat,v 1.13 2012/03/20 18:42:29 matt Exp $ */ +/* $NetBSD: nodes.c.pat,v 1.14 2018/06/22 11:04:55 kre Exp $ */ /*- * Copyright (c) 1991, 1993 @@ -35,6 +35,8 @@ */ #include <stdlib.h> +#include <stddef.h> + /* * Routine for dealing with parsed shell commands. */ @@ -46,43 +48,70 @@ #include "mystring.h" -int funcblocksize; /* size of structures in function */ -int funcstringsize; /* size of strings in node */ -pointer funcblock; /* block to allocate function from */ -char *funcstring; /* block to allocate strings from */ +/* used to accumulate sizes of nodes */ +struct nodesize { + int bsize; /* size of structures in function */ + int ssize; /* size of strings in node */ +}; + +/* provides resources for node copies */ +struct nodecopystate { + pointer block; /* block to allocate function from */ + char *string; /* block to allocate strings from */ +}; + %SIZES -STATIC void calcsize(union node *); -STATIC void sizenodelist(struct nodelist *); -STATIC union node *copynode(union node *); -STATIC struct nodelist *copynodelist(struct nodelist *); -STATIC char *nodesavestr(char *); +STATIC void calcsize(union node *, struct nodesize *); +STATIC void sizenodelist(struct nodelist *, struct nodesize *); +STATIC union node *copynode(union node *, struct nodecopystate *); +STATIC struct nodelist *copynodelist(struct nodelist *, struct nodecopystate *); +STATIC char *nodesavestr(char *, struct nodecopystate *); + +struct funcdef { + unsigned int refcount; + union node n; /* must be last */ +}; /* * Make a copy of a parse tree. */ -union node * +struct funcdef * copyfunc(union node *n) { + struct nodesize sz; + struct nodecopystate st; + struct funcdef *fn; + if (n == NULL) return NULL; - funcblocksize = 0; - funcstringsize = 0; - calcsize(n); - funcblock = ckmalloc(funcblocksize + funcstringsize); - funcstring = (char *) funcblock + funcblocksize; - return copynode(n); + sz.bsize = offsetof(struct funcdef, n); + sz.ssize = 0; + calcsize(n, &sz); + fn = ckmalloc(sz.bsize + sz.ssize); + fn->refcount = 1; + st.block = (char *)fn + offsetof(struct funcdef, n); + st.string = (char *)fn + sz.bsize; + copynode(n, &st); + return fn; } +union node * +getfuncnode(struct funcdef *fn) +{ + if (fn == NULL) + return NULL; + return &fn->n; +} STATIC void -calcsize(union node *n) +calcsize(union node *n, struct nodesize *res) { %CALCSIZE } @@ -90,11 +119,11 @@ calcsize(union node *n) STATIC void -sizenodelist(struct nodelist *lp) +sizenodelist(struct nodelist *lp, struct nodesize *res) { while (lp) { - funcblocksize += SHELL_ALIGN(sizeof(struct nodelist)); - calcsize(lp->n); + res->bsize += SHELL_ALIGN(sizeof(struct nodelist)); + calcsize(lp->n, res); lp = lp->next; } } @@ -102,7 +131,7 @@ sizenodelist(struct nodelist *lp) STATIC union node * -copynode(union node *n) +copynode(union node *n, struct nodecopystate *st) { union node *new; @@ -112,17 +141,17 @@ copynode(union node *n) STATIC struct nodelist * -copynodelist(struct nodelist *lp) +copynodelist(struct nodelist *lp, struct nodecopystate *st) { struct nodelist *start; struct nodelist **lpp; lpp = &start; while (lp) { - *lpp = funcblock; - funcblock = (char *) funcblock + + *lpp = st->block; + st->block = (char *)st->block + SHELL_ALIGN(sizeof(struct nodelist)); - (*lpp)->n = copynode(lp->n); + (*lpp)->n = copynode(lp->n, st); lp = lp->next; lpp = &(*lpp)->next; } @@ -133,27 +162,49 @@ copynodelist(struct nodelist *lp) STATIC char * -nodesavestr(char *s) +nodesavestr(char *s, struct nodecopystate *st) { register char *p = s; - register char *q = funcstring; - char *rtn = funcstring; + register char *q = st->string; + char *rtn = st->string; while ((*q++ = *p++) != 0) continue; - funcstring = q; + st->string = q; return rtn; } /* - * Free a parse tree. + * Handle making a reference to a function, and releasing it. + * Free the func code when there are no remaining references. */ void -freefunc(union node *n) +reffunc(struct funcdef *fn) +{ + if (fn != NULL) + fn->refcount++; +} + +void +unreffunc(struct funcdef *fn) +{ + if (fn != NULL) { + if (--fn->refcount > 0) + return; + ckfree(fn); + } +} + +/* + * this is used when we need to free the func, regardless of refcount + * which only happens when re-initing the shell for a SHELLPROC + */ +void +freefunc(struct funcdef *fn) { - if (n) - ckfree(n); + if (fn != NULL) + ckfree(fn); }