gconv: Check reference count in __gconv_release_cache [BZ #24677]
Commit Message
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
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.
@@ -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);
}
@@ -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
new file mode 100644
@@ -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>