Re: PR/58208 CVS commit: src

[Taylor R Campbell <riastradh%NetBSD.org@localhost>]

The attached patch fixes the remaining xfail tests by allocating a
guard page before each of the

C ctype
C compat ctype
C tolower
C toupper

tables in libc initializers.

With this patch, any attempt to pass a negative char value into the
ctype(3) functions in any locale will be detected noisily and trigger
SIGSEGV or SIGABRT, rather than yield bogus and nondeterministic
answers.

This is tempting.  But I'm not sure this is worth the cost, because
the cost is incurred at _every_ program startup with libc.  So it
needs some measurement, at least.

# HG changeset patch
# User Taylor R Campbell <riastradh%NetBSD.org@localhost>
# Date 1743186173 0
#      Fri Mar 28 18:22:53 2025 +0000
# Branch trunk
# Node ID 44cc0fea499139f79fe235456e01559239ede6b8
# Parent  2f58eca9ae23856391bac5cbd7a9dc4d8581c1c9
# EXP-Topic riastradh-pr58208-runtimectypeabusedetection
libc: Put guard pages before the C ctype/tolower/toupper tables.

This may incur some overhead from additional mmap/mprotect calls on
every program startup in libc.

This also only affects machines where char is signed for now.  (But
maybe it would be worth doing unconditionally; users could still try
to pass in explicit `signed char' inputs.)

PR lib/58208: ctype(3) provides poor runtime feedback of abuse

diff -r 2f58eca9ae23 -r 44cc0fea4991 lib/libc/citrus/citrus_lc_ctype.c
--- a/lib/libc/citrus/citrus_lc_ctype.c	Sat Mar 29 01:40:59 2025 +0000
+++ b/lib/libc/citrus/citrus_lc_ctype.c	Fri Mar 28 18:22:53 2025 +0000
@@ -102,12 +102,32 @@ static __inline void
 	_DIAGASSERT(data != NULL);
 
 	__mb_cur_max = _citrus_ctype_get_mb_cur_max(data->rl_citrus_ctype);
-	_ctype_tab_ = data->rl_ctype_tab;
-	_tolower_tab_ = data->rl_tolower_tab;
-	_toupper_tab_ = data->rl_toupper_tab;
+#ifndef __CHAR_UNSIGNED__
+	if (__predict_false(data->rl_ctype_tab == _C_ctype_tab_))
+		_ctype_tab_ = _C_ctype_tab_guarded;
+	else
+#endif
+		_ctype_tab_ = data->rl_ctype_tab;
+#ifndef __CHAR_UNSIGNED__
+	if (__predict_false(data->rl_tolower_tab == _C_tolower_tab_))
+		_tolower_tab_ = _C_tolower_tab_guarded;
+	else
+#endif
+		_tolower_tab_ = data->rl_tolower_tab;
+#ifndef __CHAR_UNSIGNED__
+	if (__predict_false(data->rl_toupper_tab == _C_toupper_tab_))
+		_toupper_tab_ = _C_toupper_tab_guarded;
+	else
+#endif
+		_toupper_tab_ = data->rl_toupper_tab;
 
 #ifdef __BUILD_LEGACY
-	_ctype_ = data->rl_compat_bsdctype;
+#ifndef __CHAR_UNSIGNED__
+	if (__predict_false(data->rl_compat_bsdctype == _C_compat_bsdctype))
+		_ctype_ = _C_compat_bsdctype_guarded;
+	else
+#endif
+		_ctype_ = data->rl_compat_bsdctype;
 #endif
 }
 
diff -r 2f58eca9ae23 -r 44cc0fea4991 lib/libc/gen/ctype_.c
--- a/lib/libc/gen/ctype_.c	Sat Mar 29 01:40:59 2025 +0000
+++ b/lib/libc/gen/ctype_.c	Fri Mar 28 18:22:53 2025 +0000
@@ -44,8 +44,14 @@
 #endif /* LIBC_SCCS and not lint */
 
 #include <sys/ctype_bits.h>
+#include <sys/mman.h>
+
 #include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
 #include "ctype_local.h"
+#include "runetype_local.h"
 
 #if EOF != -1
 #error "EOF != -1"
@@ -158,3 +164,52 @@ const unsigned short _C_ctype_tab_[1 + _
 #undef _X
 
 const unsigned short *_ctype_tab_ = &_C_ctype_tab_[0];
+
+#ifndef __CHAR_UNSIGNED__
+
+#define	roundup(X, N)	((((X) + ((N) - 1))/(N))*(N))
+
+__dso_hidden
+const void *
+guard_ctype(const void *tab, size_t size)
+{
+	const unsigned page_size = sysconf(_SC_PAGESIZE);
+	size_t nbytes = 0;
+	void *p = MAP_FAILED, *q = NULL;
+
+	nbytes = page_size + roundup(size, page_size);
+	p = mmap(NULL, nbytes, PROT_READ|PROT_WRITE, MAP_ANON,
+	    /*fd*/-1, /*offset*/0);
+	if (p == MAP_FAILED)
+		goto fail;
+	if (mprotect(p, page_size, PROT_NONE) == -1)
+		goto fail;
+	q = (char *)p + page_size;
+	memcpy(q, tab, size);
+	memset((char *)q + size, 0xff, nbytes - size - page_size);
+	return q;
+
+fail:	if (p != MAP_FAILED)
+		(void)munmap(p, nbytes);
+	return tab;
+}
+
+#ifdef __BUILD_LEGACY
+__dso_hidden const unsigned char *_C_compat_bsdctype_guarded =
+    &_C_compat_bsdctype[0];
+#endif
+__dso_hidden const unsigned short *_C_ctype_tab_guarded = &_C_ctype_tab_[0];
+
+static void __attribute__((constructor))
+ctype_guard_init(void)
+{
+
+#ifdef __BUILD_LEGACY
+	_ctype_ = _C_compat_bsdctype_guarded =
+	    guard_ctype(_C_compat_bsdctype, sizeof(_C_compat_bsdctype));
+#endif
+	_ctype_tab_ = _C_ctype_tab_guarded =
+	    guard_ctype(_C_ctype_tab_, sizeof(_C_ctype_tab_));
+}
+
+#endif	/* __CHAR_UNSIGNED__ */
diff -r 2f58eca9ae23 -r 44cc0fea4991 lib/libc/gen/tolower_.c
--- a/lib/libc/gen/tolower_.c	Sat Mar 29 01:40:59 2025 +0000
+++ b/lib/libc/gen/tolower_.c	Fri Mar 28 18:22:53 2025 +0000
@@ -61,3 +61,17 @@ const short _C_tolower_tab_[1 + _CTYPE_N
 #endif
 
 const short *_tolower_tab_ = &_C_tolower_tab_[0];
+
+#ifndef __CHAR_UNSIGNED__
+
+__dso_hidden const short *_C_tolower_tab_guarded = &_C_tolower_tab_[0];
+
+static void __attribute__((constructor))
+tolower_guard_init(void)
+{
+
+	_tolower_tab_ = _C_tolower_tab_guarded =
+	    guard_ctype(_C_tolower_tab_, sizeof(_C_tolower_tab_));
+}
+
+#endif	/* __CHAR_UNSIGNED__ */
diff -r 2f58eca9ae23 -r 44cc0fea4991 lib/libc/gen/toupper_.c
--- a/lib/libc/gen/toupper_.c	Sat Mar 29 01:40:59 2025 +0000
+++ b/lib/libc/gen/toupper_.c	Fri Mar 28 18:22:53 2025 +0000
@@ -61,3 +61,17 @@ const short _C_toupper_tab_[1 + _CTYPE_N
 #endif
 
 const short *_toupper_tab_ = &_C_toupper_tab_[0];
+
+#ifndef __CHAR_UNSIGNED__
+
+__dso_hidden const short *_C_toupper_tab_guarded = &_C_toupper_tab_[0];
+
+static void __attribute__((constructor))
+toupper_guard_init(void)
+{
+
+	_toupper_tab_ = _C_toupper_tab_guarded =
+	    guard_ctype(_C_toupper_tab_, sizeof(_C_toupper_tab_));
+}
+
+#endif	/* __CHAR_UNSIGNED__ */
diff -r 2f58eca9ae23 -r 44cc0fea4991 lib/libc/locale/ctype_local.h
--- a/lib/libc/locale/ctype_local.h	Sat Mar 29 01:40:59 2025 +0000
+++ b/lib/libc/locale/ctype_local.h	Fri Mar 28 18:22:53 2025 +0000
@@ -49,6 +49,16 @@ extern const short _C_tolower_tab_[];
 #ifdef __BUILD_LEGACY
 extern const unsigned char	*_ctype_;
 extern const unsigned char	_C_compat_bsdctype[];
+#ifndef __CHAR_UNSIGNED__
+__dso_hidden extern const unsigned char *_C_compat_bsdctype_guarded;
+#endif
+#endif
+
+#ifndef __CHAR_UNSIGNED__
+__dso_hidden const void *guard_ctype(const void *, size_t);
+__dso_hidden extern const unsigned short *_C_ctype_tab_guarded;
+__dso_hidden extern const short *_C_tolower_tab_guarded;
+__dso_hidden extern const short *_C_toupper_tab_guarded;
 #endif
 
 #endif /*_CTYPE_LOCAL_H_*/
diff -r 2f58eca9ae23 -r 44cc0fea4991 tests/lib/libc/gen/t_ctype.c
--- a/tests/lib/libc/gen/t_ctype.c	Sat Mar 29 01:40:59 2025 +0000
+++ b/tests/lib/libc/gen/t_ctype.c	Fri Mar 28 18:22:53 2025 +0000
@@ -112,13 +112,7 @@ test_abuse_in_locales(const char *name, 
 		ATF_REQUIRE_MSG(setlocale(LC_CTYPE, locales[i]) != NULL,
 		    "locales[i]=%s", locales[i]);
 		snprintf(buf, sizeof(buf), "[%s]%s", locales[i], name);
-		if (macro && strcmp(locales[i], "C") == 0) {
-			atf_tc_expect_fail("PR lib/58208: ctype(3)"
-			    " provides poor runtime feedback of abuse");
-		}
 		test_abuse(buf, ctypefn);
-		if (strcmp(locales[i], "C") == 0)
-			atf_tc_expect_pass();
 	}
 }
 
@@ -789,8 +783,6 @@ ATF_TC_BODY(abuse_##FN##_macro_c, tc)			
 		atf_tc_skip("runtime ctype(3) abuse is impossible with"	      \
 		    " unsigned char");					      \
 	}								      \
-	atf_tc_expect_fail("PR lib/58208:"				      \
-	    " ctype(3) provides poor runtime feedback of abuse");	      \
 	test_abuse(#FN, &FN##_wrapper);					      \
 }									      \
 ATF_TC(abuse_##FN##_function_c);					      \