lib/58631: libc: missing compile-time checks of mbstate_t usage

[campbell+netbsd%mumble.net@localhost]

>Number:         58631
>Category:       lib
>Synopsis:       libc: missing compile-time checks of mbstate_t usage
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Aug 24 13:00:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NetMBS Foundation_t
>Environment:
>Description:
The new mbrtoc8/16/32 and c8/16/32rtomb functions combine

(a) a four-byte buffer for UTF-8/16/32 code units (and possibly some padding), with
(b) an arbitrary locale-dependent mbstate_t for wcrtomb_l or mbrtowc_l conversion,

in a single caller-allocated mbstate_t object.
 
This requires the locale-dependent conversion logic to use no more than sizeof(mbstate_t) - offsetof(struct mbrtocNstate, mbs) bytes of storage in the mbstate_t.

Empirically, I have found that our Citrus modules don't use more than 36 bytes of mbstate_t by adding

__CTASSERT(sizeof(_ENCODING_STATE) <= 36);

to citrus_ctype_template.h, and since mbstate_t is 128 bytes long (and needs certainly no more than 64-byte alignment, more likely 8-byte at most on any architecture), we're safe for now.

But the state actually used by Citrus modules could change (and I may have missed something else that is stuffed into an mbstate_t).  So this requirement needs to be asserted somewhere, and it needs to be matched up to cNrtomb/mbrtocN.
>How-To-Repeat:
code inspection
>Fix:
A cursory first draft for experiment, not fit for commit:

diff -r cf7a8f9687ea lib/libc/citrus/citrus_ctype_template.h
--- a/lib/libc/citrus/citrus_ctype_template.h	Sat Aug 24 07:24:34 2024 +0000
+++ b/lib/libc/citrus/citrus_ctype_template.h	Sat Aug 24 12:54:58 2024 +0000
@@ -511,6 +511,9 @@ int
 __CTASSERT(alignof(_ENCODING_STATE) <= alignof(int) ||
            alignof(_ENCODING_STATE) <= alignof(void *));
 
+/* Ensure we have enough buffer space for mbrtocN(3) and cNrtomb(3) */
+__CTASSERT(sizeof(_ENCODING_STATE) <= 36);
+
 static int
 _FUNCNAME(ctype_init)(void ** __restrict cl,
 		      void * __restrict var, size_t lenvar, size_t lenps)
diff -r cf7a8f9687ea lib/libc/locale/c32rtomb.h
--- a/lib/libc/locale/c32rtomb.h	Sat Aug 24 07:24:34 2024 +0000
+++ b/lib/libc/locale/c32rtomb.h	Sat Aug 24 12:54:58 2024 +0000
@@ -34,7 +34,7 @@ struct c32rtombstate {
 	 * XXX This needs to match the maximum size of any conversion
 	 * state actually used by wcrtomb_l.
 	 */
-	char		dummy;
+	char		dummy[96];
 };
 
 #endif	/* LIB_LIBC_LOCALE_C32RTOMB_H_ */
diff -r cf7a8f9687ea lib/libc/locale/mbrtoc32.h
--- a/lib/libc/locale/mbrtoc32.h	Sat Aug 24 07:24:34 2024 +0000
+++ b/lib/libc/locale/mbrtoc32.h	Sat Aug 24 12:54:58 2024 +0000
@@ -34,7 +34,7 @@ struct mbrtoc32state {
 	 * XXX This needs to match the maximum size of any conversion
 	 * state actually used by mbrtowc_l.
 	 */
-	char		dummy;
+	char		dummy[96];
 };
 
 #endif	/* LIB_LIBC_LOCALE_MBRTOC32_H_ */