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

Message ID 87a7dl99rk.fsf@oldenburg2.str.redhat.com
State Superseded
Headers show
Series
  • gconv: Check reference count in __gconv_release_cache [BZ #24677]
Related show

Commit Message

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.

Comments

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

-- 
Yann Droneaud
OPTEYA

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>