From patchwork Thu Feb 28 06:27:40 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 10832581 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5CFA81805 for ; Thu, 28 Feb 2019 06:27:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 44E352D96D for ; Thu, 28 Feb 2019 06:27:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 37B852D97F; Thu, 28 Feb 2019 06:27:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9FDA22D96D for ; Thu, 28 Feb 2019 06:27:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725896AbfB1G1v (ORCPT ); Thu, 28 Feb 2019 01:27:51 -0500 Received: from orcrist.hmeau.com ([104.223.48.154]:39720 "EHLO deadmen.hmeau.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725899AbfB1G1u (ORCPT ); Thu, 28 Feb 2019 01:27:50 -0500 Received: from gondobar.mordor.me.apana.org.au ([192.168.128.4] helo=gondobar) by deadmen.hmeau.com with esmtps (Exim 4.89 #2 (Debian)) id 1gzFAW-0005eP-AJ; Thu, 28 Feb 2019 14:27:44 +0800 Received: from herbert by gondobar with local (Exim 4.89) (envelope-from ) id 1gzFAS-0001ZY-Ca; Thu, 28 Feb 2019 14:27:40 +0800 Date: Thu, 28 Feb 2019 14:27:40 +0800 From: Herbert Xu To: Harald van Dijk Cc: Martijn Dekker , DASH shell mailing list Subject: [PATCH] shell: Reset handler when entering a subshell Message-ID: <20190228062740.qscdy4cqb2ur4h4c@gondor.apana.org.au> References: <7f1b21c0-d3e7-2f5f-70d5-a4c51b775547@inlv.org> <60da258a-1f73-affc-b564-41fe5ed05764@gigawatt.nl> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <60da258a-1f73-affc-b564-41fe5ed05764@gigawatt.nl> User-Agent: NeoMutt/20170113 (1.7.2) Sender: dash-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dash@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jan 11, 2019 at 11:25:48PM +0000, Harald van Dijk wrote: > On 10/01/2019 14:21, Martijn Dekker wrote: > > Op 10-01-19 om 11:37 schreef Martijn Dekker: > > > In a dot script sourced with 'command .' (which is useful to avoid > > > exiting if the script doesn't exist), triggering a syntax error in an > > > 'eval' in a subshell causes dash to hang at the end of the main script. > > > > In fact, 'eval' doesn't appear related. I can also trigger the bug by > > triggering an error in another special builtin: > > > > command . /dev/stdin < > ( set -o foobar ) && echo WOOPS > > EOF > > echo end > > I do not see a hang myself, but I definitely see wrong behaviour: the output > I get is > > $ command . /dev/stdin < > ( set -o foobar ) && echo WOOPS > > EOF > src/dash: 1: set: Illegal option -o foobar > $ echo end > end > $ > WOOPS > $ > > This was introduced by > > commit 46d5a7fcea81b489819f753451c1ad2fe435f148 > Author: Herbert Xu > Date: Tue Mar 27 00:39:35 2018 +0800 > > eval: Restore input files in evalcommand > > When evalcommand invokes a command that modifies parsefile and > then bails out without popping the file, we need to ensure the > input file is restored so that the shell can continue to execute. > > What I think it going on here, although I do not understand it completely > yet, is: > > evalcommand invokes a command that modifies parsefile: that's the dot > command. The evaluation of the dotted file involves creating a subshell, and > because of the invalid option, that subshell exits with EXERROR. Because the > subshell is invoked from within the command builtin, that EXERROR is eaten, > and the input file is restored. The subshell then happily continues reading > commands at the same time as the parent shell. Thanks for the analysis. I think the real bug here is the fact that subshells can "escape" their jail by a longjmp that is caught by evalcommand. This is something that both NetBSD/FreeBSD have already fixed. Here is a similar patch to fix it in dash: ---8<--- As it is a subshell can execute code that is only meant for the parent shell when it executes a longjmp that is caught by something like evalcommand. This patch fixes it by resetting the handler when entering a subshell. Reported-by: Martijn Dekker Signed-off-by: Herbert Xu diff --git a/src/error.h b/src/error.h index 94e30a2..c5db134 100644 --- a/src/error.h +++ b/src/error.h @@ -34,6 +34,9 @@ * @(#)error.h 8.2 (Berkeley) 5/4/95 */ +#ifndef _SRC_ERROR_H +#define _SRC_ERROR_H + #include #include @@ -127,3 +130,5 @@ void exerror(int, const char *, ...) __attribute__((__noreturn__)); const char *errmsg(int, int); void sh_warnx(const char *, ...); + +#endif /* _SRC_ERROR_H */ diff --git a/src/jobs.c b/src/jobs.c index 26a6248..80ae742 100644 --- a/src/jobs.c +++ b/src/jobs.c @@ -952,9 +952,10 @@ forkshell(struct job *jp, union node *n, int mode) TRACE(("forkshell(%%%d, %p, %d) called\n", jobno(jp), n, mode)); pid = fork(); - if (pid == 0) + if (pid == 0) { + handler = &main_handler; forkchild(jp, n, mode); - else + } else forkparent(jp, n, mode, pid); return pid; diff --git a/src/main.c b/src/main.c index 6b3a090..5346ef6 100644 --- a/src/main.c +++ b/src/main.c @@ -71,6 +71,7 @@ int *dash_errno; short profile_buf[16384]; extern int etext(); #endif +struct jmploc main_handler; STATIC void read_profile(const char *); STATIC char *find_dot_file(char *); @@ -90,7 +91,6 @@ main(int argc, char **argv) { char *shinit; volatile int state; - struct jmploc jmploc; struct stackmark smark; int login; @@ -102,7 +102,7 @@ main(int argc, char **argv) monitor(4, etext, profile_buf, sizeof profile_buf, 50); #endif state = 0; - if (unlikely(setjmp(jmploc.loc))) { + if (unlikely(setjmp(main_handler.loc))) { int e; int s; @@ -137,7 +137,7 @@ main(int argc, char **argv) else goto state4; } - handler = &jmploc; + handler = &main_handler; #ifdef DEBUG opentrace(); trputs("Shell args: "); trargs(argv); diff --git a/src/main.h b/src/main.h index 19e4983..d54e10d 100644 --- a/src/main.h +++ b/src/main.h @@ -35,6 +35,7 @@ */ #include +#include "error.h" /* pid of main shell */ extern int rootpid; @@ -49,6 +50,8 @@ extern int *dash_errno; #define errno (*dash_errno) #endif +extern struct jmploc main_handler; + void readcmdfile(char *); int dotcmd(int, char **); int exitcmd(int, char **);