Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 17 Mar 2021 10:27:29 GMT
From:      Alex Richardson <arichardson@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 8dc9d6a7b09f - stable/13 - libc/qsort: Don't allow interposing recursive calls
Message-ID:  <202103171027.12HARTsX095796@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
The branch stable/13 has been updated by arichardson:

URL: https://cgit.FreeBSD.org/src/commit/?id=8dc9d6a7b09f28458c1ad2848abdeb3f944c3132

commit 8dc9d6a7b09f28458c1ad2848abdeb3f944c3132
Author:     Alex Richardson <arichardson@FreeBSD.org>
AuthorDate: 2021-02-18 10:12:29 +0000
Commit:     Alex Richardson <arichardson@FreeBSD.org>
CommitDate: 2021-03-17 09:46:58 +0000

    libc/qsort: Don't allow interposing recursive calls
    
    This causes problems when using ASAN with a runtime older than 12.0 since
    the intercept does not expect qsort() to call itself using an interposable
    function call. This results in infinite recursion and stack exhaustion
    when a binary compiled with -fsanitize=address calls qsort.
    See also https://bugs.llvm.org/show_bug.cgi?id=46832 and
    https://reviews.llvm.org/D84509 (ASAN runtime patch).
    
    To prevent this problem, this patch uses a static helper function
    for the actual qsort() implementation. This prevents interposition and
    allows for direct calls. As a nice side-effect, we can also move the
    qsort_s checks to the top-level function and out of the recursive calls.
    
    Reviewed By:    kib
    Differential Revision: https://reviews.freebsd.org/D28133
    
    (cherry picked from commit cbcfe28f9d5f975f97b7fb4a0d72bc9780eb0c46)
---
 lib/libc/stdlib/qsort.c | 98 ++++++++++++++++++++++++-------------------------
 1 file changed, 48 insertions(+), 50 deletions(-)

diff --git a/lib/libc/stdlib/qsort.c b/lib/libc/stdlib/qsort.c
index 27d03ea8e829..cfd2d99025f0 100644
--- a/lib/libc/stdlib/qsort.c
+++ b/lib/libc/stdlib/qsort.c
@@ -91,41 +91,23 @@ __unused
 	      :(CMP(thunk, b, c) > 0 ? b : (CMP(thunk, a, c) < 0 ? a : c ));
 }
 
+/*
+ * The actual qsort() implementation is static to avoid preemptible calls when
+ * recursing. Also give them different names for improved debugging.
+ */
 #if defined(I_AM_QSORT_R)
-void
-qsort_r(void *a, size_t n, size_t es, void *thunk, cmp_t *cmp)
+#define local_qsort local_qsort_r
 #elif defined(I_AM_QSORT_S)
-errno_t
-qsort_s(void *a, rsize_t n, rsize_t es, cmp_t *cmp, void *thunk)
-#else
-#define	thunk NULL
-void
-qsort(void *a, size_t n, size_t es, cmp_t *cmp)
+#define local_qsort local_qsort_s
 #endif
+static void
+local_qsort(void *a, size_t n, size_t es, cmp_t *cmp, void *thunk)
 {
 	char *pa, *pb, *pc, *pd, *pl, *pm, *pn;
 	size_t d1, d2;
 	int cmp_result;
 	int swap_cnt;
 
-#ifdef I_AM_QSORT_S
-	if (n > RSIZE_MAX) {
-		__throw_constraint_handler_s("qsort_s : n > RSIZE_MAX", EINVAL);
-		return (EINVAL);
-	} else if (es > RSIZE_MAX) {
-		__throw_constraint_handler_s("qsort_s : es > RSIZE_MAX", EINVAL);
-		return (EINVAL);
-	} else if (n != 0) {
-		if (a == NULL) {
-			__throw_constraint_handler_s("qsort_s : a == NULL", EINVAL);
-			return (EINVAL);
-		} else if (cmp == NULL) {
-			__throw_constraint_handler_s("qsort_s : cmp == NULL", EINVAL);
-			return (EINVAL);
-		}
-	}
-#endif
-
 loop:
 	swap_cnt = 0;
 	if (n < 7) {
@@ -134,11 +116,7 @@ loop:
 			     pl > (char *)a && CMP(thunk, pl - es, pl) > 0;
 			     pl -= es)
 				swapfunc(pl, pl - es, es);
-#ifdef I_AM_QSORT_S
-		return (0);
-#else
 		return;
-#endif
 	}
 	pm = (char *)a + (n / 2) * es;
 	if (n > 7) {
@@ -187,11 +165,7 @@ loop:
 			     pl > (char *)a && CMP(thunk, pl - es, pl) > 0;
 			     pl -= es)
 				swapfunc(pl, pl - es, es);
-#ifdef I_AM_QSORT_S
-		return (0);
-#else
 		return;
-#endif
 	}
 
 	pn = (char *)a + n * es;
@@ -205,13 +179,7 @@ loop:
 	if (d1 <= d2) {
 		/* Recurse on left partition, then iterate on right partition */
 		if (d1 > es) {
-#if defined(I_AM_QSORT_R)
-			qsort_r(a, d1 / es, es, thunk, cmp);
-#elif defined(I_AM_QSORT_S)
-			qsort_s(a, d1 / es, es, cmp, thunk);
-#else
-			qsort(a, d1 / es, es, cmp);
-#endif
+			local_qsort(a, d1 / es, es, cmp, thunk);
 		}
 		if (d2 > es) {
 			/* Iterate rather than recurse to save stack space */
@@ -223,13 +191,7 @@ loop:
 	} else {
 		/* Recurse on right partition, then iterate on left partition */
 		if (d2 > es) {
-#if defined(I_AM_QSORT_R)
-			qsort_r(pn - d2, d2 / es, es, thunk, cmp);
-#elif defined(I_AM_QSORT_S)
-			qsort_s(pn - d2, d2 / es, es, cmp, thunk);
-#else
-			qsort(pn - d2, d2 / es, es, cmp);
-#endif
+			local_qsort(pn - d2, d2 / es, es, cmp, thunk);
 		}
 		if (d1 > es) {
 			/* Iterate rather than recurse to save stack space */
@@ -238,8 +200,44 @@ loop:
 			goto loop;
 		}
 	}
+}
+
+#if defined(I_AM_QSORT_R)
+void
+qsort_r(void *a, size_t n, size_t es, void *thunk, cmp_t *cmp)
+{
+	local_qsort_r(a, n, es, cmp, thunk);
+}
+#elif defined(I_AM_QSORT_S)
+errno_t
+qsort_s(void *a, rsize_t n, rsize_t es, cmp_t *cmp, void *thunk)
+{
+	if (n > RSIZE_MAX) {
+		__throw_constraint_handler_s("qsort_s : n > RSIZE_MAX", EINVAL);
+		return (EINVAL);
+	} else if (es > RSIZE_MAX) {
+		__throw_constraint_handler_s("qsort_s : es > RSIZE_MAX",
+		    EINVAL);
+		return (EINVAL);
+	} else if (n != 0) {
+		if (a == NULL) {
+			__throw_constraint_handler_s("qsort_s : a == NULL",
+			    EINVAL);
+			return (EINVAL);
+		} else if (cmp == NULL) {
+			__throw_constraint_handler_s("qsort_s : cmp == NULL",
+			    EINVAL);
+			return (EINVAL);
+		}
+	}
 
-#ifdef I_AM_QSORT_S
+	local_qsort_s(a, n, es, cmp, thunk);
 	return (0);
-#endif
 }
+#else
+void
+qsort(void *a, size_t n, size_t es, cmp_t *cmp)
+{
+	local_qsort(a, n, es, cmp, NULL);
+}
+#endif

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202103171027.12HARTsX095796>