From patchwork Wed Jan 16 13:32:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 10765931 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 51BCC1390 for ; Wed, 16 Jan 2019 13:32:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4088B2CD5A for ; Wed, 16 Jan 2019 13:32:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 347CD2DD52; Wed, 16 Jan 2019 13:32:45 +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 4DDF62D542 for ; Wed, 16 Jan 2019 13:32:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732798AbfAPNcn (ORCPT ); Wed, 16 Jan 2019 08:32:43 -0500 Received: from orcrist.hmeau.com ([104.223.48.154]:45374 "EHLO deadmen.hmeau.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732780AbfAPNcn (ORCPT ); Wed, 16 Jan 2019 08:32:43 -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 1gjlJ6-0007lF-7t; Wed, 16 Jan 2019 21:32:36 +0800 Received: from herbert by gondobar with local (Exim 4.89) (envelope-from ) id 1gjlJ1-0006do-P6; Wed, 16 Jan 2019 21:32:31 +0800 Date: Wed, 16 Jan 2019 21:32:31 +0800 From: Herbert Xu To: Martijn Dekker Cc: Harald van Dijk , dash@vger.kernel.org Subject: [PATCH v3] redir: Handle nested exec within REALLY_CLOSED redirection Message-ID: <20190116133231.mx53nyjhgrfiflch@gondor.apana.org.au> References: <20180919042550.6z2rhszi4m5r6vqa@gondor.apana.org.au> <7bd61c96-8c14-aaba-d410-1ec4335a0608@inlv.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <7bd61c96-8c14-aaba-d410-1ec4335a0608@inlv.org> 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, Dec 14, 2018 at 01:01:12AM +0000, Martijn Dekker wrote: > > > So did anything happen on the bash front? I'm happy to change if > > bash moves in the same direction. > > Yes. According to the bash changelog, Chet fixed it in git last 30th of > April, meaning it'll be in bash 5.0. > > Patch attached, as per Harald's suggestion. Thanks for the patch. I took a deeper look into the history of the bug and it turned out that I added REALLY_CLOSED as an optimisation in order to avoid an unnecessary close(2) syscall. Removing REALLY_CLOSED would mean an unnecessary close(2) in such cases. So I'm going to go for a more complicated fix: ---8<--- The value of REALLY_CLOSED is used to avoid an unnecessary close(2) call when restoring redirections. However, as it stands it can remove a close(2) call that's actually needed. This happens when an enclosed exec(1) command leaves an open file descriptor behind. This patch fixes this by going up one level when popping redirections and changing REALLY_CLOSED back to CLOSED when we encounter the nested exec(1) command. Reported-by: Martijn Dekker Fixes: ce0f1900d869 ("[REDIR] Fix redirect restore on saved file...") Signed-off-by: Herbert Xu diff --git a/src/redir.c b/src/redir.c index e67cc0a..4f84a80 100644 --- a/src/redir.c +++ b/src/redir.c @@ -128,12 +128,12 @@ redirect(union node *redir, int flags) p = &sv->renamed[fd]; i = *p; - if (likely(i == EMPTY)) { - i = CLOSED; - if (fd != newfd) { + if (likely(i <= EMPTY)) { + if (i == EMPTY && fd != newfd) { i = savefd(fd, fd); fd = -1; - } + } else + i = CLOSED; } if (i == newfd) @@ -340,16 +340,20 @@ out: void popredir(int drop) { + struct redirtab *nrp; struct redirtab *rp; int i; INTOFF; rp = redirlist; + nrp = rp->next; for (i = 0 ; i < 10 ; i++) { switch (rp->renamed[i]) { case CLOSED: if (!drop) close(i); + else if (nrp && nrp->renamed[i] == REALLY_CLOSED) + nrp->renamed[i] = CLOSED; break; case EMPTY: case REALLY_CLOSED: @@ -361,7 +365,7 @@ popredir(int drop) break; } } - redirlist = rp->next; + redirlist = nrp; ckfree(rp); INTON; } @@ -451,8 +455,17 @@ struct redirtab *pushredir(union node *redir) sv = ckmalloc(sizeof (struct redirtab)); sv->next = q; redirlist = sv; - for (i = 0; i < 10; i++) - sv->renamed[i] = EMPTY; + for (i = 0; i < 10; i++) { + int val = EMPTY; + + if (q) { + val = q->renamed[i]; + if (val > EMPTY) + val = EMPTY; + } + + sv->renamed[i] = val; + } out: return q;