From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29611 invoked by alias); 24 Jul 2013 15:39:40 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 31574 Received: (qmail 17073 invoked from network); 24 Jul 2013 15:39:34 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 Received-SPF: none (ns1.primenet.com.au: domain at samsung.com does not designate permitted sender hosts) X-AuditID: cbfec7f5-b7f376d000001ec6-2d-51eff53189a4 Date: Wed, 24 Jul 2013 16:39:28 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: bug with eval, proc-subst and pipes Message-id: <20130724163928.50c84d40@pwslap01u.europe.root.pri> In-reply-to: <20130720181144.1b5a7fc0@pws-pc.ntlworld.com> References: <20130715133525.GA7694@chaz.gmail.com> <130715100624.ZM14123@torch.brasslantern.com> <20130716215540.22d88a27@pws-pc.ntlworld.com> <130717000027.ZM15643@torch.brasslantern.com> <20130717201733.2c0b029b@pws-pc.ntlworld.com> <20130718095741.3f54725f@pwslap01u.europe.root.pri> <20130718102227.527429bd@pwslap01u.europe.root.pri> <20130718123210.054f1ea1@pwslap01u.europe.root.pri> <20130719200159.GA8712@chaz.gmail.com> <20130719205853.GB8712@chaz.gmail.com> <20130720181144.1b5a7fc0@pws-pc.ntlworld.com> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupjluLIzCtJLcpLzFFi42I5/e/4FV3Dr+8DDb4stbA42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGVff32Uq2ChU8aWviaWBcRZfFyMnh4SAicTW+dfZIWwxiQv3 1rN1MXJxCAksZZTon/8BylnOJNF44Q0rSBWLgKrE0/+tYB1sAoYSUzfNZgSxRQTEJc6uPc8C YgsLGEgsbzkJFucVsJeYf+IqG4jNKWAlMfHDUmaIobdZJI5ueA7WwC+gL3H17ycmiDPsJWZe OQPVLCjxY/I9sBpmAS2JzduaWCFseYnNa94yT2AUmIWkbBaSsllIyhYwMq9iFE0tTS4oTkrP NdIrTswtLs1L10vOz93ECAnDrzsYlx6zOsQowMGoxMNbOOtdoBBrYllxZe4hRgkOZiUR3rcP 3gcK8aYkVlalFuXHF5XmpBYfYmTi4JRqYJx3heshn1LiVNermxIyrnmrhb1TYn0x0e/Zwv2T bl5MurKZZfPW+QaRMyNmyDqc6DuTee/s16zpV82ZF97lOxHja3jG2WeRhsyZpbfS+IQrZ17m +SF8Vtel81jEt/Brda1R3cV9Av6GPLxTbosoTNwcd95dZkrmM35RWfasb2GTHjZMdMviNFRi Kc5INNRiLipOBABqm4VaIQIAAA== On Sat, 20 Jul 2013 18:11:44 +0100 Peter Stephenson wrote: > On Fri, 19 Jul 2013 21:58:53 +0100 > Stephane Chazelas wrote: > > and fd 13 by closedumps() (the missing link to compinit?). > > That's distinctly suspicious, if 13 was opened for the process > substitution. > > Does the following fix it, or is there yet more? It did, apart from the other bit. > If it does we still have an error for older systems, but I suspect > FD_CLOEXEC (and O_CLOEXEC) are widely enough supported that may be more > a theoretical then an actual problem. This removes the side effects of the error in this case by properly removing the dump record instead of just trimming it inconsistently, so no one will try to use the closed file descriptor in the parent shell. However, it presumably implies there are cases where the dump is being removed where it doesn't need to be, which is the only way that could trigger reuse of the file descriptor in the dump record. The only obvious way that could happen is a failed exec without a fork. That's a slightly strange to thing to happen in your main shell: it would mean you're trying to replace it with something else but gave up and shrugged your shoulders and carried on using it. So there's presumably still some aspect I'm missing. diff --git a/Src/parse.c b/Src/parse.c index b670925..0c2a458 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -3418,6 +3418,16 @@ incrdumpcount(FuncDump f) f->count++; } +/**/ +static void +freedump(FuncDump f) +{ + munmap((void *) f->addr, f->len); + zclose(f->fd); + zsfree(f->filename); + zfree(f, sizeof(*f)); +} + /* Decrement the reference counter for a dump file. If zero, unmap the file. */ /**/ @@ -3434,10 +3444,7 @@ decrdumpcount(FuncDump f) q->next = p->next; else dumps = p->next; - munmap((void *) f->addr, f->len); - zclose(f->fd); - zsfree(f->filename); - zfree(f, sizeof(*f)); + freedump(f); } } } @@ -3447,10 +3454,11 @@ decrdumpcount(FuncDump f) mod_export void closedumps(void) { - FuncDump p; - - for (p = dumps; p; p = p->next) - zclose(p->fd); + while (dumps) { + FuncDump p = dumps->next; + freedump(dumps); + dumps = p; + } } #endif