NetBSD Problem Report #58347

From www@netbsd.org  Mon Jun 17 15:23:52 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)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id B47C71A9238
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 17 Jun 2024 15:23:52 +0000 (UTC)
Message-Id: <20240617152351.962A21A923A@mollari.NetBSD.org>
Date: Mon, 17 Jun 2024 15:23:51 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: libc ldexp has signed integer overflow UB
X-Send-Pr-Version: www-1.0

>Number:         58347
>Category:       lib
>Synopsis:       libc ldexp has signed integer overflow UB
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          needs-pullups
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 17 15:25:00 +0000 2024
>Closed-Date:    
>Last-Modified:  Mon Jun 17 18:53:55 +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).

>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58347 CVS commit: src/lib/libc/compat/gen
Date: Mon, 17 Jun 2024 18:16:58 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Mon Jun 17 18:16:58 UTC 2024

 Modified Files:
 	src/lib/libc/compat/gen: compat_ldexp_ieee754.c

 Log Message:
 libc ldexp(3): Avoid undefined behaviour in arithmetic overflow.

 PR lib/58347


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/lib/libc/compat/gen/compat_ldexp_ieee754.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->needs-pullups
State-Changed-By: riastradh@NetBSD.org
State-Changed-When: Mon, 17 Jun 2024 18:53:55 +0000
State-Changed-Why:
fix committed


>Unformatted:

Home	PR Database Search