Date: Thu, 18 Feb 2021 14:03:59 GMT From: Alex Richardson <arichardson@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: cbcfe28f9d5f - main - libc/qsort: Don't allow interposing recursive calls Message-ID: <202102181403.11IE3xJd030878@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by arichardson: URL: https://cgit.FreeBSD.org/src/commit/?id=cbcfe28f9d5f975f97b7fb4a0d72bc9780eb0c46 commit cbcfe28f9d5f975f97b7fb4a0d72bc9780eb0c46 Author: Alex Richardson <arichardson@FreeBSD.org> AuthorDate: 2021-02-18 10:12:29 +0000 Commit: Alex Richardson <arichardson@FreeBSD.org> CommitDate: 2021-02-18 14:02:48 +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 --- 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?202102181403.11IE3xJd030878>