Page MenuHomeFreeBSD

libc/qsort: Don't allow interposing recursive calls
ClosedPublic

Authored by arichardson on Jan 13 2021, 12:21 PM.
Tags
None
Referenced Files
F81628276: D28133.diff
Fri, Apr 19, 5:39 AM
Unknown Object (File)
Mar 7 2024, 10:27 PM
Unknown Object (File)
Mar 7 2024, 10:27 PM
Unknown Object (File)
Feb 4 2024, 10:16 AM
Unknown Object (File)
Dec 24 2023, 4:12 PM
Unknown Object (File)
Dec 20 2023, 8:04 AM
Unknown Object (File)
Nov 27 2023, 8:13 AM
Unknown Object (File)
Nov 22 2023, 6:09 PM
Subscribers

Details

Summary

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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 36166
Build 33055: arc lint + arc unit

Event Timeline

ping? Is this approach okay or should I add a static qsort_impl helper function instead?

I'm unsure what the consequence is of making these symbols have protected visibility. Won't this give any other nasty side effects?

  • Use a static function instead
  • Move the qsort_s checks out of recursive calls
lib/libc/stdlib/qsort.c
108

Do you ever return non-zero from local_qsort()? If not, why not make it void?

qsort_s would return (0) then.

109

I think it would be useful for debugging to name these local_qsort() functions differently for each I_AM_QSORT_X

lib/libc/stdlib/qsort.c
108

good catch, I forgot to change that after moving the qsort_s checks.

109

Yes that makes sense, will update.

change return type and rename local fns

kib added inline comments.
lib/libc/stdlib/qsort.c
242

No need.

This revision is now accepted and ready to land.Feb 16 2021, 10:58 AM