13021 Invalid state if bindtextdomain() fails during re-binding

Reviewed by: Joshua M. Clulow <josh@sysmgr.org>
Approved by: Dan McDonald <danmcd@joyent.com>

6 files changed, 181 insertions, 6 deletions

diff --git a/usr/src/lib/libc/port/i18n/gettext_real.c b/usr/src/lib/libc/port/i18n/gettext_real.c
index 6045d000fe..6e5b8054ae 100644
--- a/usr/src/lib/libc/port/i18n/gettext_real.c
+++ b/usr/src/lib/libc/port/i18n/gettext_real.c
@@ -58,7 +58,7 @@ char *
 _real_gettext_u(const char *domain, const char *msgid1, const char *msgid2,
     unsigned long int ln, int category, int plural, locale_t loc)
 {
-	char	msgfile[MAXPATHLEN]; 	/* 1024 */
+	char	msgfile[MAXPATHLEN]; /* 1024 */
 	char	mydomain[TEXTDOMAINMAX + 1]; /* 256 + 1 */
 	char	*cur_binding;	/* points to current binding in list */
 	const char *cur_locale;
@@ -326,7 +326,7 @@ static int
 process_nlspath(const char *cur_domain, const char *cur_msgloc,
     const char *nlspath, char **binding)
 {
-	char 	*s;				/* generic string ptr */
+	char	*s;			/* generic string ptr */
 	char	*territory;		/* our current territory element */
 	char	*codeset;		/* our current codeset element */
 	char	*s1;			/* for handling territory */
@@ -684,12 +684,12 @@ _real_bindtextdomain_u(const char *domain, const char *binding,
 				return (*binding_addr);
 			}
 			/* replace existing binding with new binding */
-			if (*binding_addr) {
-				free(*binding_addr);
-			}
-			if ((*binding_addr = strdup(binding)) == NULL) {
+			char *new_binding = strdup(binding);
+			if (new_binding == NULL) {
 				return (NULL);
 			}
+			free(*binding_addr);
+			*binding_addr = new_binding;
 #ifdef GETTEXT_DEBUG
 			printlist();
 #endif
diff --git a/usr/src/pkg/manifests/system-test-libctest.mf b/usr/src/pkg/manifests/system-test-libctest.mf
index 91c272f9b9..659006a358 100644
--- a/usr/src/pkg/manifests/system-test-libctest.mf
+++ b/usr/src/pkg/manifests/system-test-libctest.mf
@@ -28,6 +28,7 @@ dir path=opt/libc-tests/cfg
 dir path=opt/libc-tests/cfg/symbols
 dir path=opt/libc-tests/runfiles
 dir path=opt/libc-tests/tests
+dir path=opt/libc-tests/tests/i18n
 dir path=opt/libc-tests/tests/random
 dir path=opt/libc-tests/tests/regex
 dir path=opt/libc-tests/tests/regex/data
@@ -93,6 +94,9 @@ file path=opt/libc-tests/tests/fnmatch.64 mode=0555
 file path=opt/libc-tests/tests/fpround_test mode=0555
 file path=opt/libc-tests/tests/fpround_test.$(ARCH) mode=0555
 file path=opt/libc-tests/tests/fpround_test.$(ARCH64) mode=0555
+file path=opt/libc-tests/tests/i18n/bindtextdomain_test mode=0555
+file path=opt/libc-tests/tests/i18n/bindtextdomain_test.$(ARCH) mode=0555
+file path=opt/libc-tests/tests/i18n/bindtextdomain_test.$(ARCH64) mode=0555
 file path=opt/libc-tests/tests/memset_s.32 mode=0555
 file path=opt/libc-tests/tests/memset_s.64 mode=0555
 file path=opt/libc-tests/tests/newlocale_test mode=0555
diff --git a/usr/src/test/libc-tests/runfiles/default.run b/usr/src/test/libc-tests/runfiles/default.run
index e59f1b104e..dd1cb4cc3a 100644
--- a/usr/src/test/libc-tests/runfiles/default.run
+++ b/usr/src/test/libc-tests/runfiles/default.run
@@ -38,6 +38,8 @@ outputdir = /var/tmp/test_results
 [/opt/libc-tests/tests/wcsncasecmp-7350.32]
 [/opt/libc-tests/tests/wcsncasecmp-7350.64]
 
+[/opt/libc-tests/tests/i18n/bindtextdomain_test]
+
 [/opt/libc-tests/tests/random/getrandom]
 [/opt/libc-tests/tests/random/getentropy]
 [/opt/libc-tests/tests/random/chacha]
diff --git a/usr/src/test/libc-tests/tests/Makefile b/usr/src/test/libc-tests/tests/Makefile
index c4db6695ce..0b460ff2f2 100644
--- a/usr/src/test/libc-tests/tests/Makefile
+++ b/usr/src/test/libc-tests/tests/Makefile
@@ -18,6 +18,7 @@
 SUBDIRS = \
 	catopen \
 	fpround \
+	i18n \
 	newlocale \
 	nl_langinfo \
 	priv_gettext \
diff --git a/usr/src/test/libc-tests/tests/i18n/Makefile b/usr/src/test/libc-tests/tests/i18n/Makefile
new file mode 100644
index 0000000000..56410d23a3
--- /dev/null
+++ b/usr/src/test/libc-tests/tests/i18n/Makefile
@@ -0,0 +1,25 @@
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright 2020 Richard Hansen <rhansen@rhansen.org>
+#
+
+include $(SRC)/Makefile.master
+
+TESTSUBDIR = i18n
+PROG = bindtextdomain_test
+ARCHPROG = bindtextdomain_test
+
+include ../Makefile.com
+
+LDLIBS += -lumem
+LDLIBS64 += -lumem
diff --git a/usr/src/test/libc-tests/tests/i18n/bindtextdomain_test.c b/usr/src/test/libc-tests/tests/i18n/bindtextdomain_test.c
new file mode 100644
index 0000000000..bb608e0328
--- /dev/null
+++ b/usr/src/test/libc-tests/tests/i18n/bindtextdomain_test.c
@@ -0,0 +1,143 @@
+/*
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ */
+
+/*
+ * Copyright 2020 Richard Hansen <rhansen@rhansen.org>
+ */
+
+#include <errno.h>
+#include <libintl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/sysmacros.h>
+#include <umem.h>
+#include <unistd.h>
+#include "test_common.h"
+
+const char *
+_umem_debug_init(void)
+{
+	return ("default");
+}
+
+int
+main(int argc, char *argv[])
+{
+	int ret = 0;
+	int optc;
+	while ((optc = getopt(argc, argv, "df")) != -1) {
+		switch (optc) {
+		case 'd':
+			test_set_debug();
+			break;
+		case 'f':
+			test_set_force();
+			break;
+		default:
+			(void) fprintf(stderr, "Usage: %s [-df]\n", argv[0]);
+			exit(1);
+		}
+	}
+
+	struct {
+		const char *name;
+		const char *dir;
+		bool malloc_fail;
+		const char *want;
+		int want_errno;
+	} test_cases[] = {
+		{
+			.name = "unbound query",
+			.dir = NULL,
+			.want = "/usr/lib/locale/",
+		},
+		{
+			.name = "bind malloc fail",
+			.dir = "/bounddir1",
+			.malloc_fail = true,
+			.want = NULL,
+			.want_errno = EAGAIN,
+		},
+		{
+			.name = "query after bind malloc fail",
+			.dir = NULL,
+			.want = "/usr/lib/locale/",
+		},
+		{
+			.name = "normal bind",
+			.dir = "/bounddir2",
+			.want = "/bounddir2",
+		},
+		{
+			.name = "query after normal bind",
+			.dir = NULL,
+			.want = "/bounddir2",
+		},
+		{
+			.name = "rebind to same",
+			.dir = "/bounddir2",
+			.want = "/bounddir2",
+		},
+		{
+			.name = "query after rebind to same",
+			.dir = NULL,
+			.want = "/bounddir2",
+		},
+		{
+			.name = "rebind to new",
+			.dir = "/bounddir3",
+			.want = "/bounddir3",
+		},
+		{
+			.name = "query after rebind to new",
+			.dir = NULL,
+			.want = "/bounddir3",
+		},
+		{
+			.name = "rebind malloc fail",
+			.dir = "/bounddir4",
+			.malloc_fail = true,
+			.want = NULL,
+			.want_errno = EAGAIN,
+		},
+		{
+			.name = "query after rebind malloc fail",
+			.dir = NULL,
+			.want = "/bounddir3",
+		},
+	}, *tc;
+
+	for (size_t i = 0; i < ARRAY_SIZE(test_cases); ++i) {
+		tc = &test_cases[i];
+		test_t t = test_start(tc->name);
+		umem_setmtbf((uint_t)tc->malloc_fail);
+		errno = 0;
+		const char *got = bindtextdomain("domain", tc->dir);
+		int got_errno = errno;
+		umem_setmtbf(0);
+		if (((got == NULL) != (tc->want == NULL)) ||
+		    ((got != NULL) && strcmp(got, tc->want))) {
+			test_failed(t, "returned %s, want %s",
+			    got != NULL ? got : "<NULL>",
+			    tc->want != NULL ? tc->want : "<NULL>");
+			ret = 1;
+		}
+		if (got_errno != tc->want_errno) {
+			test_failed(t, "got errno %d, want %d",
+			    got_errno, tc->want_errno);
+			ret = 1;
+		}
+		test_passed(t);
+	}
+	test_summary();
+	return (ret);
+}