[PATCH] gconv: Check reference count in __gconv_release_cache [BZ #24677]

[Florian Weimer <fweimer at redhat dot com>]

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  <fweimer@redhat.com>

	[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/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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <locale.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+#include <wchar.h>
+
+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 <support/test-driver.c>
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