su_tagarg: fall back to the va_list copy path under AddressSanitizer#332
Open
garacil wants to merge 1 commit into
Open
su_tagarg: fall back to the va_list copy path under AddressSanitizer#332garacil wants to merge 1 commit into
garacil wants to merge 1 commit into
Conversation
The SU_HAVE_TAGSTACK fast path reads the variable arguments directly off the caller's stack (ta_start() stores tl[1] = TAG_NEXT(&(v) + 1)). When the resulting tag list is walked - e.g. tl_len() / tl_adup() following t_next() -> t_next_next() - the raw stack pointer is dereferenced below the ta stack object, which AddressSanitizer correctly reports as a stack-buffer-underflow (reported via nua_create()). Gate the stack fast path on a new SU_TAGARG_USE_TAGSTACK, which is SU_HAVE_TAGSTACK && not AddressSanitizer (GCC __SANITIZE_ADDRESS__ or clang __has_feature(address_sanitizer)), and use it for both ta_start() and ta_end(). Under ASan the existing portable va_list copy path (tl_vlist / tl_vfree) is used instead, which does not walk below the frame. Normal builds are unchanged (SU_TAGARG_USE_TAGSTACK == SU_HAVE_TAGSTACK); the ta_list layout (tagi_t tl[2]) is identical for both paths, so this is ABI-neutral. Fixes freeswitch#210.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
SU_HAVE_TAGSTACKfast path reads the variable arguments directly off the caller's stack —ta_start()storestl[1] = TAG_NEXT(&(v) + 1). When the resulting tag list is walked (e.g.tl_len()/tl_adup()followingt_next()→t_next_next()), that raw stack pointer is dereferenced below thetastack object, which AddressSanitizer correctly reports as a stack-buffer-underflow (the trace in this issue is vianua_create()).The fix does not touch
TAG_TYPE_OF()/tl_len(). It gates the stack fast path on a newSU_TAGARG_USE_TAGSTACK = SU_HAVE_TAGSTACK && !AddressSanitizer(GCC__SANITIZE_ADDRESS__/ clang__has_feature(address_sanitizer)) and uses it for bothta_start()andta_end(). Under ASan the existing portableva_listcopy path (tl_vlist/tl_vfree) is taken instead, which never walks below the frame.Normal builds are unchanged (
SU_TAGARG_USE_TAGSTACK == SU_HAVE_TAGSTACK), and theta_listlayout (tagi_t tl[2]) is identical for both paths, so this is ABI-neutral. Verified: full build is warning-free, and a standalone preprocessor check printsUSE_TAGSTACK=1for a normal GCC build andUSE_TAGSTACK=0under-fsanitize=address.Fixes #210.