lib/58347: libc ldexp has signed integer overflow UB

[campbell+netbsd%mumble.net@localhost]

>Number:         58347
>Category:       lib
>Synopsis:       libc ldexp has signed integer overflow UB
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 17 15:25:00 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current, 10, 9, ...
>Organization:
The NetLdexp Foundefinedbehaviation
>Environment:
>Description:
ldexp makes a feeble attempt to detect overflow, but it falls flat on its face in modern C where singed overflow is undefined behaviour:

    116 	/*
    117 	 * u.v is now normalized and oldexp has been adjusted if necessary.
    118 	 * Calculate the new exponent and check for underflow and overflow.
    119 	 */
    120 	newexp = oldexp + expon;
    121 
    122 	if (newexp >= DBL_EXP_INFNAN ||
    123 	    (oldexp >= 0 && expon >= DBL_EXP_INFNAN)) {
    124 		/*
    125 		 * The result overflowed; return +/-Inf.
    126 		 */
    127 		return overflow(val);

https://nxr.netbsd.org/xref/src/lib/libc/compat/gen/compat_ldexp_ieee754.c?r=1.8#116

This leads to t_ldexp failures on various architectures:

https://releng.netbsd.org/b5reports/amd64/2024/2024.06.16.19.21.46/test.html#lib_libm_t_ldexp_ldexp_overflow
https://releng.netbsd.org/b5reports/i386/2024/2024.06.16.19.21.46/test.html#lib_libm_t_ldexp_ldexp_overflow
https://releng.netbsd.org/b5reports/sparc/2024/2024.06.16.00.12.33/test.html#lib_libm_t_ldexp_ldexp_overflow
>How-To-Repeat:
1. build system with gcc12
2. atf-run /usr/tests/lib/libm/t_ldexp
>Fix:
Check for overflow safely by testing whether expon >= DBL_EXP_INFNAN - oldexp first _before_ adding oldexp + expon:

diff --git a/lib/libc/compat/gen/compat_ldexp_ieee754.c b/lib/libc/compat/gen/compat_ldexp_ieee754.c
index 8c8cb49a935c..f507a8cdd280 100644
--- a/lib/libc/compat/gen/compat_ldexp_ieee754.c
+++ b/lib/libc/compat/gen/compat_ldexp_ieee754.c
@@ -115,17 +115,31 @@ ldexp(double val, int expon)
 
 	/*
 	 * u.v is now normalized and oldexp has been adjusted if necessary.
-	 * Calculate the new exponent and check for underflow and overflow.
+	 * We have
+	 *
+	 *	0 <= oldexp <= DBL_EXP_INFNAN,
+	 *
+	 * but
+	 *
+	 *	INT_MIN <= expon <= INT_MAX.
+	 *
+	 * Check for underflow and overflow, and if none, calculate the
+	 * new exponent.
 	 */
-	newexp = oldexp + expon;
-
-	if (newexp >= DBL_EXP_INFNAN ||
-	    (oldexp >= 0 && expon >= DBL_EXP_INFNAN)) {
+	if (expon >= DBL_EXP_INFNAN - oldexp) {
 		/*
 		 * The result overflowed; return +/-Inf.
 		 */
 		return overflow(val);
-	} else if (newexp <= 0) {
+	}
+
+	/*
+	 * We now have INT_MIN <= oldexp + expon <= DBL_EXP_INFNAN <= INT_MAX,
+	 * so the arithmetic is safe.
+	 */
+	newexp = oldexp + expon;
+
+	if (newexp <= 0) {
 		/*
 		 * The output number is either denormal or underflows (see
 		 * comments in machine/ieee.h).