From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25770 invoked by alias); 10 Feb 2015 07:12: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: 34488 Received: (qmail 22184 invoked from network); 10 Feb 2015 07:12:38 -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=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:subject:date:message-id; bh=FognZtygm3m5Tmq15EyuM2A+aHm/uHkBaVW1nnP0XKk=; b=FrRuP+CZs9fxhNOfAvnHHSqo2akw12ESq27Z8jQHvZdHMK7ZHemouQuFSijBjwjyuM ypvJV5IRMh1ALJfBFnLcuRhlOn3834zBmstK4y/LqwPVcibVYluVHGt2PRYdboNH7GwU rZjzMQ69BGVddKOzfdRAEzX+uv514pamjmW5xpHmDWSAinDTel/buuVxFGxsMsw/vv2j bfQwYDnMAO3j6etsjLZqqzzWKhStJASyLLXV/SfBYMdwMjoQHyzVn3R5p2oAmW/VKP6o F17pW+LxDmfmVt9+1kBLOgIUHzNTiOOcMbGe8AG1l8jzMPWhPnAtj6WfDxlZYflBj+0q AKUQ== X-Received: by 10.180.108.103 with SMTP id hj7mr6503987wib.90.1423552354768; Mon, 09 Feb 2015 23:12:34 -0800 (PST) From: Mikael Magnusson To: zsh-workers@zsh.org Subject: PATCH: Fix use-after-free for print -zf and print -sf Date: Tue, 10 Feb 2015 08:12:26 +0100 Message-Id: <1423552346-18827-1-git-send-email-mikachu@gmail.com> X-Mailer: git-send-email 2.2.0.GIT % print -zf hi mem.c:1525: BUG: attempt to free already free storage % XY<81>^A This is only visible (for me) with --enable-zsh-mem, but the bug is always there. As the error message says, we do a double free. The reason is that while we fflush the open_memstream'd fout, which updates buf, we then after copying the pointer do fclose, which updates the pointer again and frees the old memory area. Boom. I don't know what the nicest way to handle this is. We could do what this patch does, or we could copy the contents of buf after flushing, but that would cause two extra pointless copies compared to this patch. We could also set fout = stdout to avoid changing the final if, but I recall someone complained about the extra fflush of stdout on my other patch a while back. --- Src/builtin.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Src/builtin.c b/Src/builtin.c index 19155fd..e0f6a5c 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -4526,7 +4526,8 @@ bin_print(char *name, char **args, Options ops, int func) if (OPT_ISSET(ops,'z') || OPT_ISSET(ops,'s')) { #ifdef HAVE_OPEN_MEMSTREAM putc(0, fout); - fflush(fout); + fclose(fout); + fout = NULL; #else rewind(fout); buf = (char *)zalloc(count + 1); @@ -4548,7 +4549,7 @@ bin_print(char *name, char **args, Options ops, int func) } /* Testing EBADF special-cases >&- redirections */ - if ((fout != stdout) ? (fclose(fout) != 0) : + if (fout && (fout != stdout) ? (fclose(fout) != 0) : (fflush(fout) != 0 && errno != EBADF)) { zwarnnam(name, "write error: %e", errno); ret = 1; -- 2.2.0.GIT