From e51c9c17af51e4055efb5a2cc36739d1d7ae457f Mon Sep 17 00:00:00 2001 From: "Barton E. Schaefer" Date: Sun, 29 Jan 2017 08:30:14 -0800 Subject: 40453: signal handler safety for callers of patcompile(PAT_STATIC), which is not re-entrant. --- ChangeLog | 6 ++++++ Src/Modules/zpty.c | 14 ++++++++++---- Src/Modules/zutil.c | 20 +++++++++++++++++--- Src/Zle/compctl.c | 32 +++++++++++++++++++++++++------- Src/Zle/complete.c | 10 ++++++++++ Src/Zle/computil.c | 8 ++++++++ Src/Zle/zle_hist.c | 10 ++++++---- Src/builtin.c | 37 ++++++++++++++++++++----------------- Src/cond.c | 5 +++++ Src/glob.c | 17 ++++++++++++----- Src/loop.c | 6 +++++- Src/options.c | 2 +- Src/parse.c | 1 + 13 files changed, 126 insertions(+), 42 deletions(-) diff --git a/ChangeLog b/ChangeLog index 87a3bbd56..abaa39471 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2017-01-28 Barton E. Schaefer + * 40453: Src/Modules/zpty.c, Src/Modules/zutil.c, + Src/Zle/compctl.c, Src/Zle/complete.c, Src/Zle/computil.c, + Src/Zle/zle_hist.c, Src/builtin.c, Src/cond.c, Src/glob.c, + Src/loop.c, Src/options.c, Src/parse.c: signal handler safety + for callers of patcompile(PAT_STATIC), which is not re-entrant. + * 40439: Src/zsh.h: PAT_HEAPDUP definition just for clarity 2017-01-28 Peter Stephenson diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c index 0ef753915..2c87be16f 100644 --- a/Src/Modules/zpty.c +++ b/Src/Modules/zpty.c @@ -544,7 +544,8 @@ ptyread(char *nam, Ptycmd cmd, char **args, int noblock, int mustmatch) p = dupstring(args[1]); tokenize(p); remnulargs(p); - if (!(prog = patcompile(p, PAT_STATIC, NULL))) { + /* Signals handlers might stomp PAT_STATIC */ + if (!(prog = patcompile(p, PAT_ZDUP, NULL))) { zwarnnam(nam, "bad pattern: %s", args[1]); return 1; } @@ -682,9 +683,14 @@ ptyread(char *nam, Ptycmd cmd, char **args, int noblock, int mustmatch) write_loop(1, buf, used); } - if (seen && (!prog || matchok || !mustmatch)) - return 0; - return cmd->fin + 1; + { + int ret = cmd->fin + 1; + if (seen && (!prog || matchok || !mustmatch)) + ret = 0; + if (prog) + freepatprog(prog); + return ret; + } } static int diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c index d95c0c254..19a8306b5 100644 --- a/Src/Modules/zutil.c +++ b/Src/Modules/zutil.c @@ -510,25 +510,33 @@ bin_zstyle(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) zwarnnam(nam, "too many arguments"); return 1; } + + queue_signals(); /* Protect PAT_STATIC */ + if (context) { tokenize(context); zstyle_contprog = patcompile(context, PAT_STATIC, NULL); - if (!zstyle_contprog) + if (!zstyle_contprog) { + unqueue_signals(); return 1; + } } else zstyle_contprog = NULL; if (stylename) { s = (Style)zstyletab->getnode2(zstyletab, stylename); - if (!s) + if (!s) { + unqueue_signals(); return 1; + } zstyletab->printnode(&s->node, list); } else { scanhashtable(zstyletab, 1, 0, 0, zstyletab->printnode, list); } + unqueue_signals(); return 0; } switch (args[0][1]) { @@ -675,14 +683,20 @@ bin_zstyle(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) char **vals; Patprog prog; + queue_signals(); /* Protect PAT_STATIC */ + tokenize(args[3]); if ((vals = lookupstyle(args[1], args[2])) && (prog = patcompile(args[3], PAT_STATIC, NULL))) { while (*vals) - if (pattry(prog, *vals++)) + if (pattry(prog, *vals++)) { + unqueue_signals(); return 0; + } } + + unqueue_signals(); return 1; } break; diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c index 52c6f1233..9e6ccb404 100644 --- a/Src/Zle/compctl.c +++ b/Src/Zle/compctl.c @@ -99,7 +99,7 @@ freecompctlp(HashNode hn) } /**/ -void +static void freecompctl(Compctl cc) { if (cc == &cc_default || @@ -142,7 +142,7 @@ freecompctl(Compctl cc) } /**/ -void +static void freecompcond(void *a) { Compcond cc = (Compcond) a; @@ -186,7 +186,7 @@ freecompcond(void *a) } /**/ -int +static int compctlread(char *name, char **args, Options ops, char *reply) { char *buf, *bptr; @@ -1564,6 +1564,8 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) Compctl cc = NULL; int ret = 0; + queue_signals(); + /* clear static flags */ cclist = 0; showmask = 0; @@ -1571,12 +1573,15 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) /* Parse all the arguments */ if (*argv) { /* Let's see if this is a global matcher definition. */ - if ((ret = get_gmatcher(name, argv))) + if ((ret = get_gmatcher(name, argv))) { + unqueue_signals(); return ret - 1; + } cc = (Compctl) zshcalloc(sizeof(*cc)); if (get_compctl(name, &argv, cc, 1, 0, 0)) { freecompctl(cc); + unqueue_signals(); return 1; } @@ -1604,6 +1609,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) printcompctl((cclist & COMP_LIST) ? "" : "DEFAULT", &cc_default, 0, 0); printcompctl((cclist & COMP_LIST) ? "" : "FIRST", &cc_first, 0, 0); print_gmatcher((cclist & COMP_LIST)); + unqueue_signals(); return ret; } @@ -1642,6 +1648,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) printcompctl("", &cc_first, 0, 0); if (cclist & COMP_LISTMATCH) print_gmatcher(COMP_LIST); + unqueue_signals(); return ret; } @@ -1656,6 +1663,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) compctl_process_cc(argv, cc); } + unqueue_signals(); return ret; } @@ -1667,12 +1675,18 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func)) static int bin_compcall(char *name, UNUSED(char **argv), Options ops, UNUSED(int func)) { + int ret; + if (incompfunc != 1) { zwarnnam(name, "can only be called from completion function"); return 1; } - return makecomplistctl((OPT_ISSET(ops,'T') ? 0 : CFN_FIRST) | - (OPT_ISSET(ops,'D') ? 0 : CFN_DEFAULT)); + + queue_signals(); + ret = makecomplistctl((OPT_ISSET(ops,'T') ? 0 : CFN_FIRST) | + (OPT_ISSET(ops,'D') ? 0 : CFN_DEFAULT)); + unqueue_signals(); + return ret; } /* @@ -1756,6 +1770,8 @@ ccmakehookfn(UNUSED(Hookdef dummy), struct ccmakedat *dat) int onm = nmatches, odm = diffmatches, osi = movefd(0); LinkNode n; + queue_signals(); + /* We build a copy of the list of matchers to use to make sure that this * works even if a shell function called from the completion code changes * the global matchers. */ @@ -1883,6 +1899,8 @@ ccmakehookfn(UNUSED(Hookdef dummy), struct ccmakedat *dat) } redup(osi, 0); dat->lst = 1; + + unqueue_signals(); return 0; } @@ -2044,7 +2062,7 @@ maketildelist(void) /* This does the check for compctl -x `n' and `N' patterns. */ /**/ -int +static int getcpat(char *str, int cpatindex, char *cpat, int class) { char *s, *t, *p; diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c index 48fcd4751..49b338f29 100644 --- a/Src/Zle/complete.c +++ b/Src/Zle/complete.c @@ -896,6 +896,8 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod) int i, l = arrlen(compwords), t = 0, b = 0, e = l - 1; Patprog pp; + queue_signals(); /* Protect PAT_STATIC */ + i = compcurrent - 1; if (i < 0 || i >= l) return 0; @@ -930,6 +932,9 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod) t = 0; if (t && mod) restrict_range(b, e); + + unqueue_signals(); + return t; } case CVT_PRENUM: @@ -952,6 +957,8 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod) { Patprog pp; + queue_signals(); /* Protect PAT_STATIC */ + if (!na) return 0; @@ -1036,6 +1043,9 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod) if (mod) ignore_suffix(ol - (p - compsuffix)); } + + unqueue_signals(); + return 1; } } diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c index 5b9ceec1a..325da6ddb 100644 --- a/Src/Zle/computil.c +++ b/Src/Zle/computil.c @@ -3928,6 +3928,8 @@ bin_comptry(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) if (*q) { char *qq, *qqq; + queue_signals(); + if (c) *c = '\0'; @@ -3999,6 +4001,8 @@ bin_comptry(char *nam, char **args, UNUSED(Options ops), UNUSED(int func)) } if (c) *c = ':'; + + unqueue_signals(); } } if (num) { @@ -4708,6 +4712,8 @@ cfp_add_sdirs(LinkList final, LinkList orig, char *skipped, if (!*p) continue; + queue_signals(); /* Protect PAT_STATIC */ + tokenize(f); pprog = patcompile(f, PAT_STATIC, NULL); untokenize(f); @@ -4740,6 +4746,8 @@ cfp_add_sdirs(LinkList final, LinkList orig, char *skipped, } } } + + unqueue_signals(); } } } diff --git a/Src/Zle/zle_hist.c b/Src/Zle/zle_hist.c index 434735de9..581ca4979 100644 --- a/Src/Zle/zle_hist.c +++ b/Src/Zle/zle_hist.c @@ -1220,10 +1220,12 @@ doisearch(char **args, int dir, int pattern) char *patbuf = ztrdup(sbuf); char *patstring; /* - * Do not use static pattern buffer (PAT_STATIC) since we call zle hooks, - * which might call other pattern functions. Use PAT_ZDUP instead. - * Use PAT_NOANCH because we don't need the match - * anchored to the end, even if it is at the start. + * Do not use static pattern buffer (PAT_STATIC) since we + * call zle hooks, which might call other pattern + * functions. Use PAT_ZDUP because we re-use the pattern + * in subsequent loops, so we can't pushheap/popheap. + * Use PAT_NOANCH because we don't need the match anchored + * to the end, even if it is at the start. */ int patflags = PAT_ZDUP|PAT_NOANCH; if (sbuf[0] == '^') { diff --git a/Src/builtin.c b/Src/builtin.c index 219fbc98f..394d2069e 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -539,18 +539,18 @@ bin_enable(char *name, char **argv, Options ops, int func) /* With -m option, treat arguments as glob patterns. */ if (OPT_ISSET(ops,'m')) { for (; *argv; argv++) { + queue_signals(); + /* parse pattern */ tokenize(*argv); - if ((pprog = patcompile(*argv, PAT_STATIC, 0))) { - queue_signals(); + if ((pprog = patcompile(*argv, PAT_STATIC, 0))) match += scanmatchtable(ht, pprog, 0, 0, 0, scanfunc, 0); - unqueue_signals(); - } else { untokenize(*argv); zwarnnam(name, "bad pattern : %s", *argv); returnval = 1; } + unqueue_signals(); } /* If we didn't match anything, we return 1. */ if (!match) @@ -3136,9 +3136,9 @@ bin_functions(char *name, char **argv, Options ops, int func) } else if (OPT_ISSET(ops,'m')) { /* List matching functions. */ for (; *argv; argv++) { + queue_signals(); tokenize(*argv); if ((pprog = patcompile(*argv, PAT_STATIC, 0))) { - queue_signals(); for (p = mathfuncs, q = NULL; p; q = p) { MathFunc next; do { @@ -3157,12 +3157,12 @@ bin_functions(char *name, char **argv, Options ops, int func) if (p) p = p->next; } - unqueue_signals(); } else { untokenize(*argv); zwarnnam(name, "bad pattern : %s", *argv); returnval = 1; } + unqueue_signals(); } } else if (OPT_PLUS(ops,'M')) { /* Delete functions. -m is allowed but is handled above. */ @@ -3312,11 +3312,11 @@ bin_functions(char *name, char **argv, Options ops, int func) if (OPT_ISSET(ops,'m')) { on &= ~PM_UNDEFINED; for (; *argv; argv++) { + queue_signals(); /* expand argument */ tokenize(*argv); if ((pprog = patcompile(*argv, PAT_STATIC, 0))) { /* with no options, just print all functions matching the glob pattern */ - queue_signals(); if (!(on|off) && !OPT_ISSET(ops,'X')) { scanmatchshfunc(pprog, 1, 0, DISABLED, shfunctab->printnode, pflags, expand); @@ -3336,12 +3336,12 @@ bin_functions(char *name, char **argv, Options ops, int func) } } } - unqueue_signals(); } else { untokenize(*argv); zwarnnam(name, "bad pattern : %s", *argv); returnval = 1; } + unqueue_signals(); } return returnval; } @@ -3469,11 +3469,11 @@ bin_unset(char *name, char **argv, Options ops, int func) /* with -m option, treat arguments as glob patterns */ if (OPT_ISSET(ops,'m')) { while ((s = *argv++)) { + queue_signals(); /* expand */ tokenize(s); if ((pprog = patcompile(s, PAT_STATIC, NULL))) { /* Go through the parameter table, and unset any matches */ - queue_signals(); for (i = 0; i < paramtab->hsize; i++) { for (pm = (Param) paramtab->nodes[i]; pm; pm = next) { /* record pointer to next, since we may free this one */ @@ -3486,12 +3486,12 @@ bin_unset(char *name, char **argv, Options ops, int func) } } } - unqueue_signals(); } else { untokenize(s); zwarnnam(name, "bad pattern : %s", s); returnval = 1; } + unqueue_signals(); } /* If we didn't match anything, we return 1. */ if (!match) @@ -3655,6 +3655,7 @@ bin_whence(char *nam, char **argv, Options ops, int func) pushheap(); matchednodes = newlinklist(); } + queue_signals(); for (; *argv; argv++) { /* parse the pattern */ tokenize(*argv); @@ -3664,7 +3665,6 @@ bin_whence(char *nam, char **argv, Options ops, int func) returnval = 1; continue; } - queue_signals(); if (!OPT_ISSET(ops,'p')) { /* -p option is for path search only. * * We're not using it, so search for ... */ @@ -3695,9 +3695,9 @@ bin_whence(char *nam, char **argv, Options ops, int func) scanmatchtable(cmdnamtab, pprog, 1, 0, 0, (all ? fetchcmdnamnode : cmdnamtab->printnode), printflags); - - unqueue_signals(); + run_queued_signals(); } + unqueue_signals(); if (all) { allmatched = argv = zlinklist2array(matchednodes); matchednodes = NULL; @@ -4024,11 +4024,11 @@ bin_unhash(char *name, char **argv, Options ops, int func) * "unhash -m '*'" is legal, but not recommended. */ if (OPT_ISSET(ops,'m')) { for (; *argv; argv++) { + queue_signals(); /* expand argument */ tokenize(*argv); if ((pprog = patcompile(*argv, PAT_STATIC, NULL))) { /* remove all nodes matching glob pattern */ - queue_signals(); for (i = 0; i < ht->hsize; i++) { for (hn = ht->nodes[i]; hn; hn = nhn) { /* record pointer to next, since we may free this one */ @@ -4039,12 +4039,12 @@ bin_unhash(char *name, char **argv, Options ops, int func) } } } - unqueue_signals(); } else { untokenize(*argv); zwarnnam(name, "bad pattern : %s", *argv); returnval = 1; } + unqueue_signals(); } /* If we didn't match anything, we return 1. */ if (!match) @@ -4127,18 +4127,18 @@ bin_alias(char *name, char **argv, Options ops, UNUSED(int func)) * glob patterns of aliases to display. */ if (OPT_ISSET(ops,'m')) { for (; *argv; argv++) { + queue_signals(); tokenize(*argv); /* expand argument */ if ((pprog = patcompile(*argv, PAT_STATIC, NULL))) { /* display the matching aliases */ - queue_signals(); scanmatchtable(ht, pprog, 1, flags1, flags2, ht->printnode, printflags); - unqueue_signals(); } else { untokenize(*argv); zwarnnam(name, "bad pattern : %s", *argv); returnval = 1; } + unqueue_signals(); } return returnval; } @@ -4348,10 +4348,12 @@ bin_print(char *name, char **args, Options ops, int func) zwarnnam(name, "no pattern specified"); return 1; } + queue_signals(); tokenize(*args); if (!(pprog = patcompile(*args, PAT_STATIC, NULL))) { untokenize(*args); zwarnnam(name, "bad pattern: %s", *args); + unqueue_signals(); return 1; } for (t = p = ++args; *p; p++) @@ -4359,6 +4361,7 @@ bin_print(char *name, char **args, Options ops, int func) *t++ = *p; *t = NULL; first = args; + unqueue_signals(); if (fmt && !*args) return 0; } /* compute lengths, and interpret according to -P, -D, -e, etc. */ diff --git a/Src/cond.c b/Src/cond.c index 42e9de30f..8ab019307 100644 --- a/Src/cond.c +++ b/Src/cond.c @@ -295,6 +295,8 @@ evalcond(Estate state, char *fromtest) int test, npat = state->pc[1]; Patprog pprog = state->prog->pats[npat]; + queue_signals(); + if (pprog == dummy_patprog1 || pprog == dummy_patprog2) { char *opat; int save; @@ -308,6 +310,7 @@ evalcond(Estate state, char *fromtest) if (!(pprog = patcompile(right, (save ? PAT_ZDUP : PAT_STATIC), NULL))) { zwarnnam(fromtest, "bad pattern: %s", right); + unqueue_signals(); return 2; } else if (save) @@ -316,6 +319,8 @@ evalcond(Estate state, char *fromtest) state->pc += 2; test = (pprog && pattry(pprog, left)); + unqueue_signals(); + return !(ctype == COND_STRNEQ ? !test : test); } case COND_STRLT: diff --git a/Src/glob.c b/Src/glob.c index 623e6f1d6..ff6b2583b 100644 --- a/Src/glob.c +++ b/Src/glob.c @@ -2462,13 +2462,20 @@ xpandbraces(LinkList list, LinkNode *np) int matchpat(char *a, char *b) { - Patprog p = patcompile(b, PAT_STATIC, NULL); + Patprog p; + int ret; - if (!p) { + queue_signals(); /* Protect PAT_STATIC */ + + if (!(p = patcompile(b, PAT_STATIC, NULL))) { zerr("bad pattern: %s", b); - return 0; - } - return pattry(p, a); + ret = 0; + } else + ret = pattry(p, a); + + unqueue_signals(); + + return ret; } /* do the ${foo%%bar}, ${foo#bar} stuff */ diff --git a/Src/loop.c b/Src/loop.c index ae87b2f5f..f7eae307b 100644 --- a/Src/loop.c +++ b/Src/loop.c @@ -620,7 +620,9 @@ execcase(Estate state, int do_exec) spprog = state->prog->pats + npat; pprog = NULL; pat = NULL; - + + queue_signals(); + if (isset(XTRACE)) { int htok = 0; pat = dupstring(ecrawstr(state->prog, state->pc, &htok)); @@ -657,6 +659,8 @@ execcase(Estate state, int do_exec) patok = anypatok = 1; state->pc += 2; nalts--; + + unqueue_signals(); } state->pc += 2 * nalts; if (isset(XTRACE)) { diff --git a/Src/options.c b/Src/options.c index e0b67d205..2b5795bab 100644 --- a/Src/options.c +++ b/Src/options.c @@ -647,7 +647,7 @@ bin_setopt(char *nam, char **args, UNUSED(Options ops), int isun) /* Expand the current arg. */ tokenize(s); - if (!(pprog = patcompile(s, PAT_STATIC, NULL))) { + if (!(pprog = patcompile(s, PAT_HEAPDUP, NULL))) { zwarnnam(nam, "bad pattern: %s", *args); continue; } diff --git a/Src/parse.c b/Src/parse.c index 314cc09d3..699ea49a2 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -3413,6 +3413,7 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map, for (; *names; names++) { tokenize(pat = dupstring(*names)); + /* Signal-safe here, caller queues signals */ if (!(pprog = patcompile(pat, PAT_STATIC, NULL))) { zwarnnam(nam, "bad pattern: %s", *names); close(dfd); -- cgit 1.4.1