NetBSD Problem Report #58631

From www@netbsd.org  Sat Aug 24 12:55:38 2024
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 0DE9C1A9241
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 24 Aug 2024 12:55:38 +0000 (UTC)
Message-Id: <20240824125536.D122C1A9242@mollari.NetBSD.org>
Date: Sat, 24 Aug 2024 12:55:36 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: libc: missing compile-time checks of mbstate_t usage
X-Send-Pr-Version: www-1.0

>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_ */

Home	PR Database Search