From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12923 invoked by alias); 28 Nov 2013 01:19:31 -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: 32063 Received: (qmail 26494 invoked from network); 28 Nov 2013 01:19:26 -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=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 From: Bart Schaefer Message-id: <131127171937.ZM23834@torch.brasslantern.com> Date: Wed, 27 Nov 2013 17:19:37 -0800 In-reply-to: <20131127202602.3897f501@pws-pc.ntlworld.com> Comments: In reply to Peter Stephenson "Re: PATCH: utils.c: Fix use of uninitialized memory in metafy()." (Nov 27, 8:26pm) References: <20131127180719.1ad6acf0@pwslap01u.europe.root.pri> <131127105409.ZM10472@torch.brasslantern.com> <20131127202602.3897f501@pws-pc.ntlworld.com> X-Mailer: OpenZMail Classic (0.9.2 24April2005) To: zsh-workers@zsh.org Subject: Re: PATCH: utils.c: Fix use of uninitialized memory in metafy(). MIME-version: 1.0 Content-type: text/plain; charset=us-ascii On Nov 27, 8:26pm, Peter Stephenson wrote: } Subject: Re: PATCH: utils.c: Fix use of uninitialized memory in metafy(). } } On Wed, 27 Nov 2013 10:54:09 -0800 } Bart Schaefer wrote: } > On Nov 27, 6:07pm, Peter Stephenson wrote: } > } } > } .... So if we've got only len valid bytes, not } > } null-terminated (or null-terminated by accident because the next byte } > } that isn't actually valid for the allocation happens to be null), we've } > } got no way of knowing this given the current interface. } > } > Does it actually matter? The only reason for (*e != 0) as far as I can } > tell is to be sure we've actually done (*e = '\0') at the very end of } > the whole thing [comment: "... unchanged (a terminating null character } > is appended to buf if necessary)"]. } > } > Can't we just move the *e = '\0' outside the "if" body and skip the test } > in the condition? } } Seems reasonable --- it requires the problem Simon was seeing to be in a } case that's requesting reallocation, else that assignment is going to } cause problems, but if it does cause problems we need to change the } caller. No, the problem Simon was seeing must in fact occur only in cases that have no metacharacters and are NOT requesting reallocation, otherwise the short-circuiting behavior of logical-or would never get as far as testing (*e != '\0'). But (in the current code) if the test succeeds, then we enter the block and execute *e = '\0', and if the test fails then *e == '\0' must be true. The only case in which assigning '\0' could be a (new) problem is one where the byte at buf[len] is already zero but is somehow in a part of memory to which we aren't allowed to write. Or where (len > 0 && *e && e != buf+len) but I don't see how that could happen either. We could throw in a DPUTS if you are worried. Simon's valgrind report wasn't a pointer out-of-bounds error, it was an uninitialized memory error.