From patchwork Wed Nov 29 13:39:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 120002 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp3068844qgn; Wed, 29 Nov 2017 05:40:28 -0800 (PST) X-Google-Smtp-Source: AGs4zMZmOnvIKOL4kr7aEwJ5cFfuTNJdKRvNEzcyXPIhOKjOkXMhfVBa1eEH42HB6rGnep1GXf+E X-Received: by 10.101.77.9 with SMTP id i9mr2814077pgt.39.1511962828928; Wed, 29 Nov 2017 05:40:28 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511962828; cv=none; d=google.com; s=arc-20160816; b=dNgT4bCAZ+B+0TGuiwsLs/RtuOEVNH1mB1k7fTHuxJppdCZbv3DoY57nDu1qUkWPol hZikYTYvLb1jvSEsrP1O73xxYs7xpYuS1I4gQ1hUGAxSqelZds8uswNbMZbwyQyYHoqr 5DQkImZsRpH74bfwafm7nnvZTU+De17iGPcc0QvaBkNO/uHJ2i1yYJrxk238csS/ofUH QVyfSRl/XwK+IIA1DSoUez/68z7gB5XHKScPoHQv6IqtHBOMByOIx0YaVTd8QFVrSs2I Bo9RbCGGUd73+ZEzdGrCb/+M+or9NCzLX0ZWT5XxgugRqQc5xKxDE/Th24KUFuGFzR6i Ukkg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:to:from:delivered-to:sender:list-help :list-post:list-archive:list-subscribe:list-unsubscribe:list-id :precedence:mailing-list:dkim-signature:domainkey-signature :arc-authentication-results; bh=XZ56SwJvuuqMNYVToUAymXO2VG9MGUbEwJ5Qq5Ur/qY=; b=HN9dNQmtWvH3g3LADIoosDJ/79LvGOudy252MobIfSy0+YYusEwa/ZdtngtYZ5yQme 5qO9bfIyeb0nYM7dsyyz+tTRcGhLE/Q3RgQllYmCeXr9J97veaM19E4yY1eZuZznkTqf imKia8f+V8i/rrfvGwYqeeqg2Qogva+Dkes2MCOnkDuIYOUryt+b0c3cG0bVFFusO1b5 UtmbLT6ixetVhRk3C+7767L+VG3R9aHJdXqmDP4WZfWpKql2XZogmr/DwqE0n1YhlhD3 Z/fkPYqQ1Vj8vOf27bvqiNzJXPhv/xxyBs0pGbhTF7EnKo9lgQb/nPYN8viyRXRWf9Kv 2G6w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=seAbEkI4; spf=pass (google.com: domain of libc-alpha-return-87619-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-87619-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id j11si1292968pgc.101.2017.11.29.05.40.28 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 29 Nov 2017 05:40:28 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-return-87619-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=seAbEkI4; spf=pass (google.com: domain of libc-alpha-return-87619-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-87619-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; q=dns; s= default; b=lVZVYMKsO1H1cgu5fAlTQNo9gaNd+ymoHISj2iJ+8Cm52XGBeVTbw 6ujmvEfXZyOpqC10piIwexWaDOi8OuyPZJD62+SmAaEvciGaQMoVEqpnOvbuPZP4 l75RkxCDmokOxlXc+xwEfhchow1oWCIKQ+EzodOhERMN3HteFltOoQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id; s=default; bh=WMZbBbKPdd5G0EyzXuKPzMBKt6U=; b=seAbEkI4Yp70DTA7dhLbgbUjhpGW 8EMeHvJqLugiQBiU7YMKWofIGlYm8cqnxNZHVchmrYAxBVK7W5p8dgj+iCJ9OleM 3t1q+WNVrVZY4EVQVZG2HxeB4qhX1428k3i7EFhShRgcBRxQd1F1zF6GgDMTKumy C41GCT8TfI11CP0= Received: (qmail 103795 invoked by alias); 29 Nov 2017 13:40:09 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 103725 invoked by uid 89); 29 Nov 2017 13:40:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=backup, ly, lx, Lx X-HELO: mail-qt0-f176.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=XZ56SwJvuuqMNYVToUAymXO2VG9MGUbEwJ5Qq5Ur/qY=; b=Rc0WfFWaYP1I4QLJ2P9i5CPxeFGPhmJDfNu2DLBFKp28a+N65pkXQVCLZ54nmulg/u JCQwt6SHnbP3TkkYPrMwPKqR2EgY9GQqT2iAYK0Xe64jXETXI7QSFERwmcsZxTZ/iwVT M7RR30/h9VKWWEnLOj37QgFZ/aFuiqnEknjcYIvOeKLrN7Njokjqhz99jQsDQFcdNhu/ 6vnCWZBgRk7h5cKiUs+vEbwQVUh8tRh2dn1tQXwhE1epDmPksIONHtTg/bWPVNv/LZ0U Tm+6FsVR1N7duk2l0qJ6skuLhWW+Dz9LCzs9kOjJ+HyCmYAVs4O8oKcuXFTikDNYg4YS 8gQg== X-Gm-Message-State: AJaThX6q9LqiX2jJaHiBPcQI4Dpofxrvbd2l13NjvGWHYMPagSVrY1ml m4R8RPFO55U82eoTSULAr3Oq5FlwaIk= X-Received: by 10.200.8.11 with SMTP id u11mr4613327qth.315.1511962801799; Wed, 29 Nov 2017 05:40:01 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH] libio: Free backup area when it not required (BZ#22415) Date: Wed, 29 Nov 2017 11:39:53 -0200 Message-Id: <1511962793-14533-1-git-send-email-adhemerval.zanella@linaro.org> Some libio operations fail to correctly free the backup area (created by _IO_{w}default_pbackfail on unget{w}c) resulting in either invalid buffer free operations or memory leaks. For instance, on the example provided by BZ#22415 a following fputc after a fseek to rewind the stream issues an invalid free on the buffer. It is because although _IO_file_overflow correctly (from fputc) correctly calls _IO_free_backup_area, the _IO_new_file_seekoff (called by fseek) updates the FILE internal pointers without first free the backup area (resulting in invalid values in the internal pointers). The wide version also shows an issue, but instead of accessing invalid pointers it leaks the backup memory on fseek/fputwc operation. Checked on x86_64-linux-gnu and i686-linux-gnu. * libio/Makefile (tests): Add tst-bz22415. (tst-bz22415-ENV): New rule. (generated): Add tst-bz22415.mtrace and tst-bz22415.check. (tests-special): Add tst-bz22415-mem.out. ($(objpfx)tst-bz22415-mem.out): New rule. * libio/fileops.c (_IO_new_file_seekoff): Call _IO_free_backup_area in case of a successful seek operation. * libio/wfileops.c (_IO_wfile_seekoff): Likewise. (_IO_wfile_overflow): Call _IO_free_wbackup_area in case a write buffer is required. * libio/tst-bz22415.c: New test. --- ChangeLog | 14 ++++++++ libio/Makefile | 11 ++++-- libio/fileops.c | 3 ++ libio/tst-bz22415.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++ libio/wfileops.c | 4 +++ 5 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 libio/tst-bz22415.c -- 2.7.4 diff --git a/libio/Makefile b/libio/Makefile index 9d09bd8..70b5530 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -62,7 +62,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ - tst-ftell-append tst-fputws + tst-ftell-append tst-fputws tst-bz22415 ifeq (yes,$(build-shared)) # Add test-fopenloc only if shared library is enabled since it depends on # shared localedata objects. @@ -151,9 +151,11 @@ tst_wprintf2-ARGS = "Some Text" test-fmemopen-ENV = MALLOC_TRACE=$(objpfx)test-fmemopen.mtrace tst-fopenloc-ENV = MALLOC_TRACE=$(objpfx)tst-fopenloc.mtrace +tst-bz22415-ENV = MALLOC_TRACE=$(objpfx)tst-bz22415.mtrace generated += test-fmemopen.mtrace test-fmemopen.check generated += tst-fopenloc.mtrace tst-fopenloc.check +generated += tst-bz22415.mtrace tst-bz22415.check aux := fileops genops stdfiles stdio strops @@ -167,7 +169,8 @@ shared-only-routines = oldiofopen oldiofdopen oldiofclose oldfileops \ oldiofsetpos64 ifeq ($(run-built-tests),yes) -tests-special += $(objpfx)test-freopen.out $(objpfx)test-fmemopen-mem.out +tests-special += $(objpfx)test-freopen.out $(objpfx)test-fmemopen-mem.out \ + $(objpfx)tst-bz22415-mem.out ifeq (yes,$(build-shared)) # Run tst-fopenloc-cmp.out and tst-openloc-mem.out only if shared # library is enabled since they depend on tst-fopenloc.out. @@ -218,3 +221,7 @@ $(objpfx)test-fmemopen-mem.out: $(objpfx)test-fmemopen.out $(objpfx)tst-fopenloc-mem.out: $(objpfx)tst-fopenloc.out $(common-objpfx)malloc/mtrace $(objpfx)tst-fopenloc.mtrace > $@; \ $(evaluate-test) + +$(objpfx)tst-bz22415-mem.out: $(objpfx)tst-bz22415.out + $(common-objpfx)malloc/mtrace $(objpfx)tst-bz22415.mtrace > $@; \ + $(evaluate-test) diff --git a/libio/fileops.c b/libio/fileops.c index a9e948f..72a0cb5 100644 --- a/libio/fileops.c +++ b/libio/fileops.c @@ -994,6 +994,9 @@ _IO_new_file_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) goto dumb; } } + + _IO_free_backup_area (fp); + /* At this point, dir==_IO_seek_set. */ /* If destination is within current buffer, optimize: */ diff --git a/libio/tst-bz22415.c b/libio/tst-bz22415.c new file mode 100644 index 0000000..d7b23fe --- /dev/null +++ b/libio/tst-bz22415.c @@ -0,0 +1,97 @@ +/* Check static buffer handling with setvbuf (BZ #22415) + + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +#include +#include + +static int +do_test (void) +{ + mtrace (); + + char *temp_file; + TEST_VERIFY_EXIT (create_temp_file ("tst-bz22145.", &temp_file)); + + char buf[BUFSIZ]; + + { + /* Check if backup buffer is correctly freed and changing back + to normal buffer does not trigger an invalid free in case of + static buffer set by setvbuf. */ + + FILE *f = fopen (temp_file, "w+b"); + TEST_VERIFY_EXIT (f != NULL); + + TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0); + TEST_VERIFY_EXIT (ungetc ('x', f) == 'x'); + TEST_VERIFY_EXIT (fseek (f, 0L, SEEK_SET) == 0); + TEST_VERIFY_EXIT (fputc ('y', f) == 'y'); + + TEST_VERIFY_EXIT (fclose (f) == 0); + } + + { + /* Check if backup buffer is correctly freed and changing back + to normal buffer does not trigger an invalid free in case of + static buffer set by setvbuf. */ + + FILE *f = fopen (temp_file, "w+b"); + TEST_VERIFY_EXIT (f != NULL); + + TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0); + TEST_VERIFY_EXIT (ungetc ('x', f) == 'x'); + TEST_VERIFY_EXIT (fputc ('y', f) == 'y'); + + TEST_VERIFY_EXIT (fclose (f) == 0); + } + + { + FILE *f = fopen (temp_file, "w+b"); + TEST_VERIFY_EXIT (f != NULL); + + TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0); + TEST_VERIFY_EXIT (ungetwc (L'x', f) == L'x'); + TEST_VERIFY_EXIT (fseek (f, 0L, SEEK_SET) == 0); + TEST_VERIFY_EXIT (fputwc (L'y', f) == L'y'); + + TEST_VERIFY_EXIT (fclose (f) == 0); + } + + { + FILE *f = fopen (temp_file, "w+b"); + TEST_VERIFY_EXIT (f != NULL); + + TEST_VERIFY_EXIT (setvbuf (f, buf, _IOFBF, BUFSIZ) == 0); + TEST_VERIFY_EXIT (ungetwc (L'x', f) == L'x'); + TEST_VERIFY_EXIT (fputwc (L'y', f) == L'y'); + + TEST_VERIFY_EXIT (fclose (f) == 0); + } + + free (temp_file); + + return 0; +} + +#include diff --git a/libio/wfileops.c b/libio/wfileops.c index 8756b6f..91139e1 100644 --- a/libio/wfileops.c +++ b/libio/wfileops.c @@ -421,6 +421,7 @@ _IO_wfile_overflow (_IO_FILE *f, wint_t wch) if (f->_wide_data->_IO_write_base == 0) { _IO_wdoallocbuf (f); + _IO_free_wbackup_area (f); _IO_wsetg (f, f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base, f->_wide_data->_IO_buf_base); @@ -850,6 +851,9 @@ _IO_wfile_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) goto dumb; } } + + _IO_free_wbackup_area (fp); + /* At this point, dir==_IO_seek_set. */ /* If destination is within current buffer, optimize: */