From patchwork Thu Jul 18 16:21:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 33731 Received: (qmail 115476 invoked by alias); 18 Jul 2019 16:21:41 -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 115461 invoked by uid 89); 18 Jul 2019 16:21:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.1 spammy=Yann X-HELO: mx1.redhat.com From: Florian Weimer To: libc-alpha@sourceware.org Subject: [PATCH] gconv: Check reference count in __gconv_release_cache [BZ #24677] Date: Thu, 18 Jul 2019 18:21:36 +0200 Message-ID: <878ssvz52n.fsf@oldenburg2.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 This fixes a regression introduced in commit 7e740ab2e7be7d83b75513aa406e0b10875f7f9c ("libio: Fix gconv-related memory leak [BZ #24583]"). __gconv_release_cache is only ever called with heap-allocated arrays which contain at least one member. The statically allocated ASCII steps are filtered out by __wcsmbs_close_conv. [This variant cleans up __gconv_release_cache somewhat and includes the test modification suggested by Yann. The test does not work due to bug 24824. I think installing all locales unconditionally is too costly timewise. Suggestions how to proceed are very welcome.] 2019-07-18 Florian Weimer [BZ #24677] * iconv/gconv_cache.c (__gconv_release_cache): Check reference counter before freeing array. * libio/Makefile (tests-container): Add tst-wfile-fclose-exit. * libio/tst-wfile-fclose-exit.c: New file. * libio/tst-wfile-fclose-exit.root/postclean.req: Likewise. diff --git a/libio/tst-wfile-fclose-exit.root/postclean.req b/libio/tst-wfile-fclose-exit.root/postclean.req new file mode 100644 index 0000000000..e69de29bb2 diff --git a/iconv/gconv_cache.c b/iconv/gconv_cache.c index 9a456bf825..4db7287cee 100644 --- a/iconv/gconv_cache.c +++ b/iconv/gconv_cache.c @@ -446,9 +446,12 @@ __gconv_lookup_cache (const char *toset, const char *fromset, void __gconv_release_cache (struct __gconv_step *steps, size_t nsteps) { - if (gconv_cache != NULL) - /* The only thing we have to deallocate is the record with the - steps. */ + /* The only thing we have to deallocate is the record with the + steps. But do not do this if the reference counter is still + positive. This can happen if the steps array was cloned by + __wcsmbs_clone_conv. (The array elements have separate __counter + fields, but they are only out of sync temporarily.) */ + if (gconv_cache != NULL && steps->__counter == 0) free (steps); } diff --git a/libio/Makefile b/libio/Makefile index 6e594b8ec5..3a4fb2f10e 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -68,9 +68,9 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \ tst-wfile-sync tst-wfile-gconv -# This test tests interaction with the gconv cache. Setting -# GCONV_CACHE during out-of-container testing disables the cache. -tests-container += tst-wfile-ascii +# These tests interact with the gconv cache. Setting GCONV_CACHE +# during out-of-container testing disables the cache. +tests-container += tst-wfile-ascii tst-wfile-fclose-exit tests-internal = tst-vtables tst-vtables-interposed tst-readline @@ -230,6 +230,7 @@ $(objpfx)tst-widetext.out: $(gen-locales) $(objpfx)tst_wprintf2.out: $(gen-locales) $(objpfx)tst-wfile-sync.out: $(gen-locales) $(objpfx)tst-wfile-gconv.out: $(gen-locales) +$(objpfx)tst-wfile-fclose-exit.out: $(gen-locales) endif $(objpfx)test-freopen.out: test-freopen.sh $(objpfx)test-freopen diff --git a/libio/tst-wfile-fclose-exit.c b/libio/tst-wfile-fclose-exit.c new file mode 100644 index 0000000000..e9f19b5155 --- /dev/null +++ b/libio/tst-wfile-fclose-exit.c @@ -0,0 +1,73 @@ +/* Check that it is possible to call fclose on wide default streams. + Copyright (C) 2019 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 +#include +#include + +static int +do_test (void) +{ + /* The test-in-container framework sets these environment variables. + The presence of GCONV_PATH invalidates this test. */ + unsetenv ("GCONV_PATH"); + unsetenv ("LOCPATH"); + + /* Create the gconv module cache. iconvconfig is in /sbin, which is + not on PATH. */ + { + char *iconvconfig = xasprintf ("%s/iconvconfig", support_sbindir_prefix); + TEST_COMPARE (system (iconvconfig), 0); + free (iconvconfig); + } + + /* Build the en_US.UTF-8 locale. */ + TEST_COMPARE (system ("localedef -f UTF-8 -i en_US en_US.UTF-8"), 0); + + if (setlocale (LC_ALL, "en_US.UTF-8") == NULL) + FAIL_EXIT1 ("setlocale: %m"); + + char *path; + xclose (create_temp_file ("tst-wfile-fclose-exit-", &path)); + support_write_file_string (path, "test file\n"); + + /* Create a stream and put it into wide mode. */ + FILE *fp = xfopen (path, "r"); + wint_t wc = fgetwc (fp); + TEST_COMPARE (wc, 't'); + + /* Make sure that stdout is fully initialized and in wide mode. */ + wprintf (L"info: character read: %d\n", (int) wc); + + xfclose (fp); + xfclose (stdin); + xfclose (stdout); + xfclose (stderr); + free (path); + + exit (0); + + return 0; +} + +#include