Skip to content

Commit

Permalink
folly: ASAN-exempt scanHaystackBlock, to avoid an FP buffer overrun
Browse files Browse the repository at this point in the history
Summary:
scanHaystackBlock may read-overrun the needle.data() buffer by
up to 15 bytes, but that overrun will never cross a page boundary.
The fix is to turn off ASAN-checking for this function, but since
that attribute is accompanied by a "noinline" one (which conflicts
with the function's own "inline"), I have also removed the "inline"
attribute on both decl and defn.  That is a good thing, regardless:
these days, there are very few cases in which we should be trying to
tell the compiler to inline.

Test Plan:
Before, this would elicit an ASAN abort.  Now it passes 100%:

fbconfig --platform-all=gcc-4.8.1-glibc-2.17 --sanitize=address \
folly/test:range_test && fbmake runtests

Reviewed By: philipp@fb.com

FB internal diff: D1162982
  • Loading branch information
meyering authored and sgolemon committed Feb 7, 2014
1 parent 7224b63 commit 219c697
Showing 1 changed file with 12 additions and 7 deletions.
19 changes: 12 additions & 7 deletions folly/Range.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,20 +172,25 @@ size_t qfind_first_byte_of_byteset(const StringPiece& haystack,
#if FOLLY_HAVE_EMMINTRIN_H && __GNUC_PREREQ(4, 6)

template <bool HAYSTACK_ALIGNED>
inline size_t scanHaystackBlock(const StringPiece& haystack,
const StringPiece& needles,
int64_t idx)
size_t scanHaystackBlock(const StringPiece& haystack,
const StringPiece& needles,
int64_t idx)
// inline is okay because it's only called from other sse4.2 functions
__attribute__ ((__target__("sse4.2")));
__attribute__ ((__target__("sse4.2")))
// Turn off ASAN because the "arr2 = ..." assignment in the loop below reads
// up to 15 bytes beyond end of the buffer in #needles#. That is ok because
// ptr2 is always 16-byte aligned, so the read can never span a page boundary.
// Also, the extra data that may be read is never actually used.
FOLLY_DISABLE_ADDRESS_SANITIZER;

// Scans a 16-byte block of haystack (starting at blockStartIdx) to find first
// needle. If HAYSTACK_ALIGNED, then haystack must be 16byte aligned.
// If !HAYSTACK_ALIGNED, then caller must ensure that it is safe to load the
// block.
template <bool HAYSTACK_ALIGNED>
inline size_t scanHaystackBlock(const StringPiece& haystack,
const StringPiece& needles,
int64_t blockStartIdx) {
size_t scanHaystackBlock(const StringPiece& haystack,
const StringPiece& needles,
int64_t blockStartIdx) {
DCHECK_GT(needles.size(), 16); // should handled by *needles16() method
DCHECK(blockStartIdx + 16 <= haystack.size() ||
(PAGE_FOR(haystack.data() + blockStartIdx) ==
Expand Down

0 comments on commit 219c697

Please sign in to comment.