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

login
register
mail settings
Submitter Florian Weimer
Date July 10, 2019, 3:42 p.m.
Message ID <87a7dl99rk.fsf@oldenburg2.str.redhat.com>
Download mbox | patch
Permalink /patch/33659/
State Superseded
Headers show

Comments

Florian Weimer - July 10, 2019, 3:42 p.m.
This fixes a regression introduced in commit
7e740ab2e7be7d83b75513aa406e0b10875f7f9c ("libio: Fix gconv-related
memory leak [BZ #24583]").

(The test depends on the test-in-containers fix for installed locales,
which is currently under discussion.)

2019-07-10  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.
Yann Droneaud - July 11, 2019, 8:43 p.m.
Hi,

Le mercredi 10 juillet 2019 à 17:42 +0200, Florian Weimer a écrit :
> This fixes a regression introduced in commit
> 7e740ab2e7be7d83b75513aa406e0b10875f7f9c ("libio: Fix gconv-related
> memory leak [BZ #24583]").
> 
> (The test depends on the test-in-containers fix for installed locales,
> which is currently under discussion.)
> 
> 2019-07-10  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.
> 

[...]

> diff --git a/libio/tst-wfile-fclose-exit.c b/libio/tst-wfile-fclose-exit.c
> new file mode 100644
> index 0000000000..9e519fb95e
> --- /dev/null
> +++ b/libio/tst-wfile-fclose-exit.c
> @@ -0,0 +1,70 @@
> +/* Check that it is possible to call fclose on 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);
> +  }
> +
> +  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);
> +  exit (0);
> +

If exit happen here, why free(path) below ?

> +  free (path);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Regards.

Patch

diff --git a/iconv/gconv_cache.c b/iconv/gconv_cache.c
index 9a456bf825..1b2500046e 100644
--- a/iconv/gconv_cache.c
+++ b/iconv/gconv_cache.c
@@ -446,9 +446,13 @@  __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 != NULL && nsteps > 0
+      && 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..9e519fb95e
--- /dev/null
+++ b/libio/tst-wfile-fclose-exit.c
@@ -0,0 +1,70 @@ 
+/* Check that it is possible to call fclose on 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);
+  }
+
+  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);
+  exit (0);
+
+  free (path);
+
+  return 0;
+}
+
+#include <support/test-driver.c>