This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] iconv_open: Fix heap corruption on gconv_init failure [BZ #22026]
- From: Florian Weimer <fweimer at redhat dot com>
- To: Andreas Schwab <schwab at suse dot de>
- Cc: libc-alpha at sourceware dot org
- Date: Tue, 29 Aug 2017 17:28:06 +0200
- Subject: Re: [PATCH] iconv_open: Fix heap corruption on gconv_init failure [BZ #22026]
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=fweimer at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5D69C4ACA7
- References: <20170829145554.680BF4015E8BE@oldenburg.str.redhat.com> <mvm1snujtv2.fsf@suse.de>
On 08/29/2017 05:06 PM, Andreas Schwab wrote:
> On Aug 29 2017, fweimer@redhat.com (Florian Weimer) wrote:
>
>> diff --git a/iconv/gconv_db.c b/iconv/gconv_db.c
>> index b748467de5..de3e5d92fe 100644
>> --- a/iconv/gconv_db.c
>> +++ b/iconv/gconv_db.c
>> @@ -318,9 +318,12 @@ gen_steps (struct derivation_step *best, const char *toset,
>> if (__builtin_expect (status, __GCONV_OK) != __GCONV_OK)
>> {
>> failed = 1;
>> - /* Make sure we unload this modules. */
>> - --step_cnt;
>> + /* Do not call the end function because the init
>> + function has failed. */
>> result[step_cnt].__end_fct = NULL;
>> +# ifdef PTR_MANGLE
>> + PTR_MANGLE (result[step_cnt].__end_fct);
>> +# endif
>
> You still need to decrement step_cnt, otherwise you have a resource
> leak.
Like this?
Thanks,
Florian
iconv_open: Fix heap corruption on gconv_init failure [BZ #22026]
Also mangle the __end_fct function pointer on the error handling
path.
2017-08-29 Florian Weimer <fweimer@redhat.com>
[BZ #22026]
* iconv/gconv_db.c (gen_steps): Decrement step_cnt after resetting
__end_fct. Mangle __end_fct after setting it to NULL.
* iconv/Makefile (tests): Add tst-gconv-init-failure.
(modules-names, modules-names-tests): Add
tst-gconv-init-failure-mod.
(gconv-modules): New target.
(tst-gconv-init-failure-mod.so): Link against libsupport.
(tst-gconv-init-failure): Depend on gconv-modules,
tst-gconv-init-failure-mod.so.
* iconv/tst-gconv-init-failure-mod.c: New file.
* iconv/tst-gconv-init-failure.c: Likewise.
* iconv/test-gconv-modules: Likewise.
diff --git a/iconv/Makefile b/iconv/Makefile
index b2fead0479..fd3575178e 100644
--- a/iconv/Makefile
+++ b/iconv/Makefile
@@ -61,6 +61,20 @@ ifeq ($(run-built-tests),yes)
xtests-special += $(objpfx)test-iconvconfig.out
endif
+# Make a copy of the file because gconv module names are constructed
+# relative to the path of the configuration file.
+$(objpfx)gconv-modules: test-gconv-modules
+ cp $< $@
+
+ifeq (yes,$(build-shared))
+tests += tst-gconv-init-failure
+modules-names += tst-gconv-init-failure-mod
+modules-names-tests += tst-gconv-init-failure-mod
+$(objpfx)tst-gconv-init-failure-mod.so: $(libsupport)
+$(objpfx)tst-gconv-init-failure.out: \
+ $(objpfx)gconv-modules $(objpfx)tst-gconv-init-failure-mod.so
+endif
+
include ../Rules
$(inst_bindir)/iconv: $(objpfx)iconv_prog $(+force)
diff --git a/iconv/gconv_db.c b/iconv/gconv_db.c
index 7a95aeaeac..1f8293672e 100644
--- a/iconv/gconv_db.c
+++ b/iconv/gconv_db.c
@@ -318,9 +318,13 @@ gen_steps (struct derivation_step *best, const char *toset,
if (__builtin_expect (status, __GCONV_OK) != __GCONV_OK)
{
failed = 1;
- /* Make sure we unload this modules. */
- --step_cnt;
+ /* Do not call the end function because the init
+ function has failed. */
result[step_cnt].__end_fct = NULL;
+# ifdef PTR_MANGLE
+ PTR_MANGLE (result[step_cnt].__end_fct);
+# endif
+ --step_cnt;
break;
}
}
diff --git a/iconv/test-gconv-modules b/iconv/test-gconv-modules
new file mode 100644
index 0000000000..edacd8cb1d
--- /dev/null
+++ b/iconv/test-gconv-modules
@@ -0,0 +1,23 @@
+# Test modules for gconv.
+# Copyright (C) 2017 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/>.
+
+# To activate these modules, tests need to put a directory with the
+# modules and a copy of this file (under the name gconv-modules) on
+# GCONV_PATH.
+
+module TST-GCONV-INIT-FAILURE// UTF-8// tst-gconv-init-failure-mod
diff --git a/iconv/tst-gconv-init-failure-mod.c b/iconv/tst-gconv-init-failure-mod.c
new file mode 100644
index 0000000000..7e7d1b9a15
--- /dev/null
+++ b/iconv/tst-gconv-init-failure-mod.c
@@ -0,0 +1,49 @@
+/* Test gconv module for tst-gconv-init-failure.
+ Copyright (C) 2017 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 <errno.h>
+#include <gconv.h>
+#include <support/check.h>
+#include <support/support.h>
+
+int
+gconv (struct __gconv_step *step,
+ struct __gconv_step_data *data,
+ const unsigned char **inptrp,
+ const unsigned char *inend,
+ unsigned char **outbufstart, size_t *irreversible,
+ int do_flush, int consume_incomplete)
+{
+ FAIL_EXIT1 ("gconv called");
+ return __GCONV_INTERNAL_ERROR;
+}
+
+int
+gconv_init (struct __gconv_step *ignored)
+{
+ write_message ("info: gconv_init called, returning error\n");
+ errno = ENOMEM;
+ return __GCONV_NOMEM;
+}
+
+int
+gconv_end (struct __gconv_step *ignored)
+{
+ FAIL_EXIT1 ("gconv_end called");
+ return __GCONV_INTERNAL_ERROR;
+}
diff --git a/iconv/tst-gconv-init-failure.c b/iconv/tst-gconv-init-failure.c
new file mode 100644
index 0000000000..92dfbd4ccd
--- /dev/null
+++ b/iconv/tst-gconv-init-failure.c
@@ -0,0 +1,58 @@
+/* Check that module __end_fct is not invoked when the init function fails.
+ Copyright (C) 2017 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 <errno.h>
+#include <iconv.h>
+#include <libgen.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <sys/auxv.h>
+
+/* Test GCONV_PATH to the directory containing the program
+ executable. */
+static void
+activate_test_gconv_modules (void)
+{
+ unsigned long ptr = getauxval (AT_EXECFN);
+ if (ptr == 0)
+ {
+ printf ("warning: AT_EXECFN not support, cannot run test\n");
+ exit (EXIT_UNSUPPORTED);
+ }
+ char *test_program_directory = dirname (xstrdup ((const char *) ptr));
+ TEST_VERIFY (setenv ("GCONV_PATH", test_program_directory, 1) == 0);
+ free (test_program_directory);
+}
+
+static int
+do_test (void)
+{
+ activate_test_gconv_modules ();
+
+ TEST_VERIFY (iconv_open ("UTF-8", "tst-gconv-init-failure//")
+ == (iconv_t) -1);
+ if (errno != ENOMEM)
+ FAIL_EXIT1 ("unexpected iconv_open error: %m");
+
+ return 0;
+}
+
+#include <support/test-driver.c>