Add an optional table lookup after the existing logarithm computation
for MidSize < Size <= MaxSize during size -> class lookups. The lookup is
O(1) due to indexing a precomputed (via constexpr) table based on a size
table. Switch to this approach for the Android size class maps.
Other approaches considered:
- Binary search was found to have an unacceptable (~30%) performance cost.
- An approach using NEON instructions (see older version of D73824) was found
to be slightly slower than this approach on newer SoCs but significantly
slower on older ones.
By selecting the values in the size tables to minimize wastage (for example,
by passing the malloc_info output of a target program to the included
compute_size_class_config program), we can increase the density of allocations
at a small (~0.5% on bionic malloc_sql_trace as measured using an identity
table) performance cost.
Reduces RSS on specific Android processes as follows (KB):
Before After
zygote (median of 50 runs) 26836 26792 (-0.2%)
zygote64 (median of 50 runs) 30384 30076 (-1.0%)
dex2oat (median of 3 runs) 375792 372952 (-0.8%)
I also measured the amount of whole-system idle dirty heap on Android by
rebooting the system and then running the following script repeatedly until
the results were stable:
for i in $(seq 1 50); do grep -A5 scudo: /proc/*/smaps | grep Pss: | cut -d: -f2 | awk '{s+=$1} END {print s}' ; sleep 1; done
I did this 3 times both before and after this change and the results were:
Before: 365650, 356795, 372663
After: 344521, 356328, 342589
These results are noisy so it is hard to make a definite conclusion, but
there does appear to be a significant effect.
On other platforms, increase the sizes of all size classes by a fixed offset
equal to the size of the allocation header. This has also been found to improve
density, since it is likely for allocation sizes to be a power of 2, which
would otherwise waste space by pushing the allocation into the next size class.
Differential Revision: https://reviews.llvm.org/D73824
This lets us remove two pointer indirections (one by removing the pointer,
and another by making the AllocatorPtr declaration hidden) in the C++ wrappers.
Differential Revision: https://reviews.llvm.org/D74356
Summary:
This tweaks some behaviors of the allocator wrt 32-bit, notably
tailoring the size-class map.
I had to remove a `printStats` from `__scudo_print_stats` since when
within Bionic they share the same slot so they can't coexist at the
same time. I have to find a solution for that later, but right now we
are not using the Svelte configuration.
Reviewers: rengolin
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74178
The class is only used in SizeClassAllocator32 in 64-bit mode, but we don't
use that class in 64-bit mode.
Differential Revision: https://reviews.llvm.org/D74099
Summary:
Forewarning: This patch looks big in #LOC changed. I promise it's not that bad, it just moves a lot of content from one file to another. I've gone ahead and left inline comments on Phabricator for sections where this has happened.
This patch:
1. Introduces the crash handler API (crash_handler_api.h).
2. Moves information required for out-of-process crash handling into an AllocatorState. This is a trivially-copied POD struct that designed to be recovered from a deceased process, and used by the crash handler to create a GWP-ASan report (along with the other trivially-copied Metadata struct).
3. Implements the crash handler API using the AllocatorState and Metadata.
4. Adds tests for the crash handler.
5. Reimplements the (now optionally linked by the supporting allocator) in-process crash handler (i.e. the segv handler) using the new crash handler API.
6. Minor updates Scudo & Scudo Standalone to fix compatibility.
7. Changed capitalisation of errors (e.g. /s/Use after free/Use After Free).
Reviewers: cryptoad, eugenis, jfb
Reviewed By: eugenis
Subscribers: merge_guards_bot, pcc, jfb, dexonsmith, mgorny, cryptoad, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73557
Summary:
I tried to move the `madvise` calls outside of one of the secondary
mutexes, but this backfired. There is situation when a low release
interval is set combined with secondary pressure that leads to a race:
a thread can get a block from the cache, while another thread is
`madvise`'ing that block, resulting in a null header.
I changed the secondary race test so that this situation would be
triggered, and moved the release into the cache mutex scope.
Reviewers: cferris, pcc, eugenis, hctim, morehouse
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D74072
By subtracting 1 from Size at the beginning we can simplify the
subsequent calculations. This also saves 4 instructions on aarch64
and 9 instructions on x86_64, but seems to be perf neutral.
Differential Revision: https://reviews.llvm.org/D73936
As a result of recent changes to the Android size classes, the malloc_free_loop
benchmark started exhausting the 8192 size class at 32768 iterations. To avoid
this problem (and to make the test more realistic), change the benchmark to
use a variety of size classes.
Differential Revision: https://reviews.llvm.org/D73918
Summary:
This changes a couple of parameters in the default Android config to
address some performance and memory footprint issues (well to be closer
to the default Bionic allocator numbers).
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73750
Summary:
The Secondary's cache needs to be released when the Combined's
`releaseToOS` function is called (via `M_PURGE`) for example,
which this CL adds.
Additionally, if doing a forced release, we'll release the
transfer batch class as well since now we can do that.
There is a couple of other house keeping changes as well:
- read the page size only once in the Secondary Cache `store`
- remove the interval check for `CanRelease`: we are going to
make that configurable via `mallopt` so this needs not be
set in stone there.
Reviewers: cferris, hctim, pcc, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73730
Summary:
A couple of seemingly innocuous changes ended up having a large impact
on the 32-bit performance. I still have to make those configurable at
some point, but right now it will have to do.
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73658
Summary:
Zygote & children's stderr is lost, so use Bionic's provided allocation
free syslog function for `outputRaw`. Get rid of the mutex as it's not
vital and could cause issues with `fork`.
Reviewers: cferris, pcc, eugenis, hctim, morehouse
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73561
Summary:
This CL changes multiple things to improve performance (notably on
Android).We introduce a cache class for the Secondary that is taking
care of this mechanism now.
The changes:
- change the Secondary "freelist" to an array. By keeping free secondary
blocks linked together through their headers, we were keeping a page
per block, which isn't great. Also we know touch less pages when
walking the new "freelist".
- fix an issue with the freelist getting full: if the pattern is an ever
increasing size malloc then free, the freelist would fill up and
entries would not be used. So now we empty the list if we get to many
"full" events;
- use the global release to os interval option for the secondary: it
was too costly to release all the time, particularly for pattern that
are malloc(X)/free(X)/malloc(X). Now the release will only occur
after the selected interval, when going through the deallocate path;
- allow release of the `BatchClassId` class: it is releasable, we just
have to make sure we don't mark the batches containing batches
pointers as free.
- change the default release interval to 1s for Android to match the
current Bionic allocator configuration. A patch is coming up to allow
changing it through `mallopt`.
- lower the smallest class that can be released to `PageSize/64`.
Reviewers: cferris, pcc, eugenis, morehouse, hctim
Subscribers: phosek, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73507
Summary:
This is an Android-specific interface for iterating over all live
allocations in a memory range.
Reviewers: hctim, cferris
Subscribers: mgorny, mgrang, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73305
Summary:
* Implement enable() and disable() in GWP-ASan.
* Setup atfork handler.
* Improve test harness sanity and re-enable GWP-ASan in Scudo.
Scudo_standalone disables embedded GWP-ASan as necessary around fork().
Standalone GWP-ASan sets the atfork handler in init() if asked to. This
requires a working malloc(), therefore GWP-ASan initialization in Scudo
is delayed to the post-init callback.
Test harness changes are about setting up a single global instance of
the GWP-ASan allocator so that pthread_atfork() does not create
dangling pointers.
Test case shamelessly stolen from D72470.
Reviewers: cryptoad, hctim, jfb
Subscribers: mgorny, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73294
Summary:
Unity is making irresponsible assumptions as to how clumped up memory
should be. With larger regions, we break those, resulting in errors
like:
"Using memoryadresses from more that 16GB of memory"
This is unfortunately one of those situations where we have to bend to
existing code because we doubt it's going to change any time soon.
128MB should be enough, but we could be flirting with OOMs in the
higher class sizes.
Reviewers: cferris, eugenis, hctim, morehouse, pcc
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73143
Summary:
In some configuration, `sched_getaffinity` can fail. Some reasons for
that being the lack of `CAP_SYS_NICE` capability or some syscall
filtering and so on.
This should not be fatal to the allocator, so in this situation, we
will fallback to the `MaxTSDCount` value specified in the allocator
configuration.
Reviewers: cferris, eugenis, hctim, morehouse, pcc
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D73055
When the hardware and operating system support the ARM Memory Tagging
Extension, tag primary allocation granules with a random tag. The granules
either side of the allocation are tagged with tag 0, which is normally
excluded from the set of tags that may be selected randomly. Memory is
also retagged with a random tag when it is freed, and we opportunistically
reuse the new tag when the block is reused to reduce overhead. This causes
linear buffer overflows to be caught deterministically and non-linear buffer
overflows and use-after-free to be caught probabilistically.
This feature is currently only enabled for the Android allocator
and depends on an experimental Linux kernel branch available here:
https://github.com/pcc/linux/tree/android-experimental-mte
All code that depends on the kernel branch is hidden behind a macro,
ANDROID_EXPERIMENTAL_MTE. This is the same macro that is used by the Android
platform and may only be defined in non-production configurations. When the
userspace interface is finalized the code will be updated to use the stable
interface and all #ifdef ANDROID_EXPERIMENTAL_MTE will be removed.
Differential Revision: https://reviews.llvm.org/D70762
Summary:
fork() wasn't well (or at all) supported in Scudo. This materialized
in deadlocks in children.
In order to properly support fork, we will lock the allocator pre-fork
and unlock it post-fork in parent and child. This is done via a
`pthread_atfork` call installing the necessary handlers.
A couple of things suck here: this function allocates - so this has to
be done post initialization as our init path is not reentrance, and it
doesn't allow for an extra pointer - so we can't pass the allocator we
are currently working with.
In order to work around this, I added a post-init template parameter
that gets executed once the allocator is initialized for the current
thread. Its job for the C wrappers is to install the atfork handlers.
I reorganized a bit the impacted area and added some tests, courtesy
of cferris@ that were deadlocking prior to this fix.
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D72470
Summary:
Some Android builds that we are interested in define `__BIONIC__`
but not `__ANDROID__`, so expand `SCUDO_ANDROID` to encompass those.
Reviewers: cferris, hctim, pcc, eugenis, morehouse
Subscribers: krytarowski, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71772
Summary:
In order to implement `malloc_{enable|disable}` we were just disabling
(or really locking) the Primary and the Secondary. That meant that
allocations could still be serviced from the TSD as long as the cache
wouldn't have to be filled from the Primary.
This wasn't working out for Android tests, so this change implements
registry disabling (eg: locking) so that `getTSDAndLock` doesn't
return a TSD if the allocator is disabled. This also means that the
Primary doesn't have to be disabled in this situation.
For the Shared Registry, we loop through all the TSDs and lock them.
For the Exclusive Registry, we add a `Disabled` boolean to the Registry
that forces `getTSDAndLock` to use the Fallback TSD instead of the
thread local one. Disabling the Registry is then done by locking the
Fallback TSD and setting the boolean in question (I don't think this
needed an atomic variable but I might be wrong).
I clang-formatted the whole thing as usual hence the couple of extra
whiteline changes in this CL.
Reviewers: cferris, pcc, hctim, morehouse, eugenis
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71719
Turns out that gtest in LLVM is only 1.8.0 (the newest version 1.10.0)
supports the GTEST_SKIP() macro, and apparently I didn't build w/o
GWP-ASan.
Should fix the GN bot, as well as any bots that may spuriously break on
platforms where the code wasn't correctly ifdef'd out as well.
Summary:
Adds GWP-ASan to Scudo standalone. Default parameters are pulled across from the
GWP-ASan build. No backtrace support as of yet.
Reviewers: cryptoad, eugenis, pcc
Reviewed By: cryptoad
Subscribers: merge_guards_bot, mgorny, #sanitizers, llvm-commits, cferris, vlad.tsyrklevich, pcc
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71229
Instead of testing whether the pointer is aligned, just align it
unconditionally and compare it to the original pointer.
This moves the computation of UserPtr up to before we start preparing the
header, so that the memory tagging code will be able to read the original
header containing the bounds of the previous allocation before it gets
potentially clobbered by the pointer realignment code.
Differential Revision: https://reviews.llvm.org/D71292
With tag-on-free we will need to get the chunk of a deallocated block. Change
getChunkFromBlock() so that it doesn't check that the chunk is allocated,
and move the check into the caller, so that it can be reused for this purpose.
Differential Revision: https://reviews.llvm.org/D71291
Summary:
The function was only defined for x86 and arm families, which ends
up being an issue for PPC in g3.
Define the function, simply returning `false` for "other"
architectures.
Reviewers: hctim, pcc, cferris, eugenis, vitalybuka
Subscribers: kristof.beyls, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D71223
The test ScudoWrappersCTest.Realloc expects realloc of memalign to work on
Android, but this relies on dealloc_type_mismatch being set to false. Commit
0d3d4d3b0 caused us to start setting it to true in the C wrapper tests,
which broke the test. Set it to the correct value on Android.
Differential Revision: https://reviews.llvm.org/D71078
The Android headers don't provide a declaration of valloc or pvalloc, so we
need to declare them ourselves.
Differential Revision: https://reviews.llvm.org/D71077
Summary:
In order to be compliant with tcmalloc's extension ownership
determination function, we have to expose a function that will
say if a chunk was allocated by us.
As to whether or not this has security consequences: someone
able to call this function repeatedly could use it to determine
secrets (cookie) or craft a valid header. So this should not be
exposed directly to untrusted user input.
Add related tests.
Additionally clang-format caught a few things to change.
Reviewers: hctim, pcc, cferris, eugenis, vitalybuka
Subscribers: JDevlieghere, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70908
This test was previously effectively doing:
P = malloc(X); write X bytes to P; P = realloc(P, X - Y); P = realloc(P, X)
and expecting that all X bytes stored to P would still be identical after
the final realloc.
This happens to be true for the current scudo implementation of realloc,
but is not guaranteed to be true by the C standard ("Any bytes in the new
object beyond the size of the old object have indeterminate values.").
This implementation detail will change with the new memory tagging support,
which unconditionally zeros newly allocated granules when memory tagging
is enabled. Fix this by limiting the number of bytes that we test to the
minimum size that we realloc the allocation to.
Differential Revision: https://reviews.llvm.org/D70761
The macros INLINE and COMPILER_CHECK always expand to the same thing (inline
and static_assert respectively). Both expansions are standards compliant C++
and are used consistently in the rest of LLVM, so let's improve consistency
with the rest of LLVM by replacing them with the expansions.
Differential Revision: https://reviews.llvm.org/D70793
Otherwise, we will hit a use-after-free when testing multiple instances of
the same allocator on the same thread. This only recently became a problem
with D70552 which caused us to run both ScudoCombinedTest.BasicCombined and
ScudoCombinedTest.ReleaseToOS on the unit tests' main thread.
Differential Revision: https://reviews.llvm.org/D70760
Summary:
This CL makes unit tests compatible with Fuchsia's zxtest. This
required a few changes here and there, but also unearthed some
incompatibilities that had to be addressed.
A header is introduced to allow to account for the zxtest/gtest
differences, some `#if SCUDO_FUCHSIA` are used to disable incompatible
code (the 32-bit primary, or the exclusive TSD).
It also brought to my attention that I was using
`__scudo_default_options` in different tests, which ended up in a
single binary, and I am not sure how that ever worked. So move
this to the main cpp.
Additionally fully disable the secondary freelist on Fuchsia as we do
not track VMOs for secondary allocations, so no release possible.
With some modifications to Scudo's BUILD.gn in Fuchsia:
```
[==========] 79 tests from 23 test cases ran (10280 ms total).
[ PASSED ] 79 tests
```
Reviewers: mcgrathr, phosek, hctim, pcc, eugenis, cferris
Subscribers: srhines, jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70682
Summary:
cferris@ found an issue where calling `releaseToOS` prior to any other
heap operation would lead to a crash, due to the allocator not being
properly initialized (it was discovered via `mallopt`).
The fix is to call `initThreadMaybe` prior to calling `releaseToOS` for
the Primary.
Add a test that crashes prior to fix.
Reviewers: hctim, cferris, pcc, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70552
Summary:
A few small improvements and optimizations:
- when refilling the free list, push back the last batch and return
the front one: this allows to keep the allocations towards the front
of the region;
- instead of using 48 entries in the shuffle array, use a multiple of
`MaxNumCached`;
- make the maximum number of batches to create on refil a constant;
ultimately it should be configurable, but that's for later;
- `initCache` doesn't need to zero out the cache, it's already done.
- it turns out that when using `||` or `&&`, the compiler is adamant
on adding a short circuit for every part of the expression. Which
ends up making somewhat annoying asm with lots of test and
conditional jump. I am changing that to bitwise `|` or `&` in two
place so that the generated code looks better. Added comments since
it might feel weird to people.
This yields to some small performance gains overall, nothing drastic
though.
Reviewers: hctim, morehouse, cferris, eugenis
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70452
When we're not building libc's allocator, just use a regular TLS variable. This
lets the unit tests pass on Android devices whose libc uses Scudo. Otherwise
libc's copy of Scudo and the unit tests' copy will both try to use the same
TLS slot, in likely incompatible ways.
This requires using ELF TLS, so start passing -fno-emulated-tls when building
the library and the unit tests on Android.
Differential Revision: https://reviews.llvm.org/D70472
Summary:
`SCUDO_DEBUG` was not enabled for unit tests, meaning the `DCHECK`s
were never tripped. While turning this on, I discovered that a few
of those not-exercised checks were actually wrong. This CL addresses
those incorrect checks.
Not that to work in tests `CHECK_IMPL` has to explicitely use the
`scudo` namespace. Also changes a C cast to a C++ cast.
Reviewers: hctim, pcc, cferris, eugenis, vitalybuka
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70276
Summary: Bionic was modified to have all function names consistent. Modify the code and get rid of the special case for bionic since it's no longer needed.
Reviewers: cryptoad
Reviewed By: cryptoad
Subscribers: srhines, llvm-commits, #sanitizers
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D70031
Summary:
cferris@ found an issue due to the new Secondary free list behavior
and unfortunately it's completely my fault. The issue is twofold:
- I lost track of the (major) fact that the Combined assumes that
all chunks returned by the Secondary are zero'd out apprioriately
when dealing with `ZeroContents`. With the introduction of the
freelist, it's no longer the case as there can be a small portion
of memory between the header and the next page boundary that is
left untouched (the rest is zero'd via release). So the next time
that block is returned, it's not fully zero'd out.
- There was no test that would exercise that behavior :(
There are several ways to fix this, the one I chose makes the most
sense to me: we pass `ZeroContents` to the Secondary's `allocate`
and it zero's out the block if requested and it's coming from the
freelist. The prevents an extraneous `memset` in case the block
comes from `map`. Another possbility could have been to `memset`
in `deallocate`, but it's probably overzealous as all secondary
blocks don't need to be zero'd out.
Add a test that would have found the issue prior to fix.
Reviewers: morehouse, hctim, cferris, pcc, eugenis, vitalybuka
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69675
Summary:
The secondary allocator is slow, because we map and unmap each block
on allocation and deallocation.
While I really like the security benefits of such a behavior, this
yields very disappointing performance numbers on Android for larger
allocation benchmarks.
So this change adds a free list to the secondary, that will hold
recently deallocated chunks, and (currently) release the extraneous
memory. This allows to save on some memory mapping operations on
allocation and deallocation. I do not think that this lowers the
security of the secondary, but can increase the memory footprint a
little bit (RSS & VA).
The maximum number of blocks the free list can hold is templatable,
`0U` meaning that we fallback to the old behavior. The higher that
number, the higher the extra memory footprint.
I added default configurations for all our platforms, but they are
likely to change in the near future based on needs and feedback.
Reviewers: hctim, morehouse, cferris, pcc, eugenis, vitalybuka
Subscribers: mgorny, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69570
Summary:
Apparently during the review of D69265, and my flailing around with
git, a somewhat important line disappeared.
On top of that, there was no test exercising that code path, and
while writing the follow up patch I intended to write, some `CHECK`s
were failing.
Re-add the missing line, and add a test that fails without said line.
Reviewers: hctim, morehouse, pcc, cferris
Reviewed By: hctim
Subscribers: #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69529
Summary:
This is a clean patch using the last diff of D69265, but using git
instead of svn, since svn went ro and arc was making my life harded
than it needed to be.
I was going to introduce a couple more lists and realized that our
lists are currently a bit all over the place. While we have a singly
linked list type relatively well defined, we are using doubly linked
lists defined on the fly for the stats and for the secondary blocks.
This CL adds a doubly linked list object, reorganizing the singly list
one to extract as much of the common code as possible. We use this
new type in the stats and the secondary. We also reorganize the list
tests to benefit from this consolidation.
There are a few side effect changes such as using for iterator loops
that are, in my opinion, cleaner in a couple of places.
Reviewers: hctim, morehouse, pcc, cferris
Reviewed By: hctim
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D69516
Android now allocates only 8 fixed TLS slots. Somehow we were getting away
with using a non-existent slot until now, but in some cases the TLS slots
were being placed at the end of a page, which led to a segfault at startup.
Differential Revision: https://reviews.llvm.org/D69191
llvm-svn: 375276
Summary:
Following up on D68471, this CL introduces some `getStats` APIs to
gather statistics in char buffers (`ScopedString` really) instead of
printing them out right away. Ultimately `printStats` will just
output the buffer, but that allows us to potentially do some work
on the intermediate buffer, and can be used for a `mallocz` type
of functionality. This allows us to pretty much get rid of all the
`Printf` calls around, but I am keeping the function in for
debugging purposes.
This changes the existing tests to use the new APIs when required.
I will add new tests as suggested in D68471 in another CL.
Reviewers: morehouse, hctim, vitalybuka, eugenis, cferris
Reviewed By: morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D68653
llvm-svn: 374173
Summary:
There was an issue in `releaseToOSMaybe`: one of the criteria to
decide if we should proceed with the release was wrong. Namely:
```
const uptr N = Sci->Stats.PoppedBlocks - Sci->Stats.PushedBlocks;
if (N * BlockSize < PageSize)
return; // No chance to release anything.
```
I meant to check if the amount of bytes in the free list was lower
than a page, but this actually checks if the amount of **in use** bytes
was lower than a page.
The correct code is:
```
const uptr BytesInFreeList =
Region->AllocatedUser -
(Region->Stats.PoppedBlocks - Region->Stats.PushedBlocks) * BlockSize;
if (BytesInFreeList < PageSize)
return 0; // No chance to release anything.
```
Consequences of the bug:
- if a class size has less than a page worth of in-use bytes (allocated
or in a cache), reclaiming would not occur, whatever the amount of
blocks in the free list; in real world scenarios this is unlikely to
happen and be impactful;
- if a class size had less than a page worth of free bytes (and enough
in-use bytes, etc), then reclaiming would be attempted, with likely
no result. This means the reclaiming was overzealous at times.
I didn't have a good way to test for this, so I changed the prototype
of the function to return the number of bytes released, allowing to
get the information needed. The test added fails with the initial
criteria.
Another issue is that `ReleaseToOsInterval` can actually be 0, meaning
we always try to release (side note: it's terrible for performances).
so change a `> 0` check to `>= 0`.
Additionally, decrease the `CanRelease` threshold to `PageSize / 32`.
I still have to make that configurable but I will do it at another time.
Finally, rename some variables in `printStats`: I feel like "available"
was too ambiguous, so change it to "total".
Reviewers: morehouse, hctim, eugenis, vitalybuka, cferris
Reviewed By: morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D68471
llvm-svn: 373930
Summary:
Initially, our malloc_info was returning ENOTSUP, but Android would
rather have it return successfully and write a barebone XML to the
stream, so we will oblige.
Add an associated test.
Reviewers: cferris, morehouse, hctim, eugenis, vitalybuka
Reviewed By: morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D68427
llvm-svn: 373754
Summary:
This changes a few things to improve memory footprint and performances
on Android, and fixes a test compilation error:
- add `stdlib.h` to `wrappers_c_test.cc` to address
https://bugs.llvm.org/show_bug.cgi?id=42810
- change Android size class maps, based on benchmarks, to improve
performances and lower the Svelte memory footprint. Also change the
32-bit region size for said configuration
- change the `reallocate` logic to reallocate in place for sizes larger
than the original chunk size, when they still fit in the same block.
This addresses patterns from `memory_replay` dumps like the following:
```
202: realloc 0xb48fd000 0xb4930650 12352
202: realloc 0xb48fd000 0xb48fd000 12420
202: realloc 0xb48fd000 0xb48fd000 12492
202: realloc 0xb48fd000 0xb48fd000 12564
202: realloc 0xb48fd000 0xb48fd000 12636
202: realloc 0xb48fd000 0xb48fd000 12708
202: realloc 0xb48fd000 0xb48fd000 12780
202: realloc 0xb48fd000 0xb48fd000 12852
202: realloc 0xb48fd000 0xb48fd000 12924
202: realloc 0xb48fd000 0xb48fd000 12996
202: realloc 0xb48fd000 0xb48fd000 13068
202: realloc 0xb48fd000 0xb48fd000 13140
202: realloc 0xb48fd000 0xb48fd000 13212
202: realloc 0xb48fd000 0xb48fd000 13284
202: realloc 0xb48fd000 0xb48fd000 13356
202: realloc 0xb48fd000 0xb48fd000 13428
202: realloc 0xb48fd000 0xb48fd000 13500
202: realloc 0xb48fd000 0xb48fd000 13572
202: realloc 0xb48fd000 0xb48fd000 13644
202: realloc 0xb48fd000 0xb48fd000 13716
202: realloc 0xb48fd000 0xb48fd000 13788
...
```
In this situation we were deallocating the old chunk, and
allocating a new one for every single one of those, but now we can
keep the same chunk (we just updated the header), which saves some
heap operations.
Reviewers: hctim, morehouse, vitalybuka, eugenis, cferris, rengolin
Reviewed By: morehouse
Subscribers: srhines, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D67293
llvm-svn: 371628
Summary:
cferris's Bionic tests found an issue in Scudo's `malloc_iterate`.
We were inclusive of both boundaries, which resulted in a `Block` that
was located on said boundary to be possibly accounted for twice, or
just being accounted for while iterating on regions that are not ours
(usually the unmapped ones in between Primary regions).
The fix is to exclude the upper boundary in `iterateOverChunks`, and
add a regression test.
This additionally corrects a typo in a comment, and change the 64-bit
Primary iteration function to not assume that `BatchClassId` is 0.
Reviewers: cferris, morehouse, hctim, vitalybuka, eugenis
Reviewed By: hctim
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D66231
llvm-svn: 369400
Summary:
Android requires additional stats in mallinfo. While we can provide
right away the number of bytes mapped (Primary+Secondary), there was
no way to get the number of free bytes (only makes sense for the
Primary since the Secondary unmaps everything on deallocation).
An approximation could be `StatMapped - StatAllocated`, but since we
are mapping in `1<<17` increments for the 64-bit Primary, it's fairly
inaccurate.
So we introduce `StatFree` (note it's `Free`, not `Freed`!), which
keeps track of the amount of Primary blocks currently unallocated.
Reviewers: cferris, eugenis, vitalybuka, hctim, morehouse
Reviewed By: morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D66112
llvm-svn: 368866
Summary:
Few corrections with no functional change:
- replacing `%zd` with `%zu` all around: the values are unsigned
- prefer `MAP_ANONYMOUS` to `MAP_ANON` (it's deprecated)
- remove the unused `enum LinkerInitialized`
- mark a parameter as `UNUSED` in Fuchsia's `getRandom`
- correct the casing of a variable and use `nullptr` instead of 0 for
pointers in `list.h`
- reorder some `typedef` to be consistent between `signed` and
`unsigned`
Reviewers: eugenis, vitalybuka, morehouse, hctim
Reviewed By: vitalybuka, morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D65660
llvm-svn: 368585
Summary:
This introduces a bunch of small optimizations with the purpose of
making the fastpath tighter:
- tag more conditions as `LIKELY`/`UNLIKELY`: as a rule of thumb we
consider that every operation related to the secondary is unlikely
- attempt to reduce the number of potentially extraneous instructions
- reorganize the `Chunk` header to not straddle a word boundary and
use more appropriate types
Note that some `LIKELY`/`UNLIKELY` impact might be less obvious as
they are in slow paths (for example in `secondary.cc`), but at this
point I am throwing a pretty wide net, and it's consistant and doesn't
hurt.
This was mosly done for the benfit of Android, but other platforms
benefit from it too. An aarch64 Android benchmark gives:
- before:
```
BM_youtube/min_time:15.000/repeats:4/manual_time_mean 445244 us 659385 us 4
BM_youtube/min_time:15.000/repeats:4/manual_time_median 445007 us 658970 us 4
BM_youtube/min_time:15.000/repeats:4/manual_time_stddev 885 us 1332 us 4
```
- after:
```
BM_youtube/min_time:15.000/repeats:4/manual_time_mean 415697 us 621925 us 4
BM_youtube/min_time:15.000/repeats:4/manual_time_median 415913 us 622061 us 4
BM_youtube/min_time:15.000/repeats:4/manual_time_stddev 990 us 1163 us 4
```
Additional since `-Werror=conversion` is enabled on some platforms we
are built on, enable it upstream to catch things early: a few sign
conversions had slept through and needed additional casting.
Reviewers: hctim, morehouse, eugenis, vitalybuka
Reviewed By: vitalybuka
Subscribers: srhines, mgorny, javed.absar, kristof.beyls, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D64664
llvm-svn: 366918
Summary:
A few corrections:
- rename `TransferBatch::MaxCached` to `getMaxCached` to conform with
the style guide;
- move `getBlockBegin` from `Chunk::` to `Allocator::`: I believe it
was a fallacy to have this be a `Chunk` method, as chunks'
relationship to backend blocks are up to the frontend allocator. It
makes more sense now, particularly with regard to the offset. Update
the associated chunk test as the method isn't available there
anymore;
- add a forgotten `\n` to a log string;
- for `releaseToOs`, instead of starting at `1`, start at `0` and
`continue` on `BatchClassId`: in the end it's identical but doesn't
assume a particular class id for batches;
- change a `CHECK` to a `reportOutOfMemory`: it's a clearer message
Reviewers: hctim, morehouse, eugenis, vitalybuka
Reviewed By: hctim
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D64570
llvm-svn: 365816
Summary:
We ran into a problem on Fuchsia where yielding threads would never
be deboosted, ultimately resulting in several threads spinning on the
same TSD, and no possibility for another thread to be scheduled,
dead-locking the process.
While this was fixed in Zircon, this lead to discussions about if
spinning without a break condition was a good decision, and settled on
a new hybrid model that would spin for a while then block.
Currently we are using a number of iterations for spinning that is
mostly arbitrary (based on sanitizer_common values), but this can
be tuned in the future.
Since we are touching `common.h`, we also use this change as a vehicle
for an Android optimization (the page size is fixed in Bionic, so use
a fixed value too).
Reviewers: morehouse, hctim, eugenis, dvyukov, vitalybuka
Reviewed By: hctim
Subscribers: srhines, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D64358
llvm-svn: 365790
Summary:
Some clang versions (< 6.0) do not inline the atomic builtin functions
leaving unresolved references to `__atomic_load_8` and so on (seems to
be mostly 64-bit atomics on 32-bit platforms).
I tried without success to use some cmake magic to detect when that
would be the case, and decided to fall back to unconditionally
linking libatomic.
Reviewers: morehouse, eugenis, vitalybuka, hctim, tejohnson
Reviewed By: tejohnson
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D64134
llvm-svn: 365052
Summary:
In some setups, using `-fsized-deallocation` would end up not finding
a sized delete operator at link time. For now, avoid using the flag
and declare the sized delete operator in the cpp test only.
This is a tentative fix as I do not have the failing setup.
Reviewers: rnk, morehouse, hctim, eugenis, vitalybuka
Reviewed By: rnk, hctim
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D64086
llvm-svn: 365045
Summary:
Adds allocation and deallocation stack trace support to Scudo. The
default provided backtrace library for GWP-ASan is supplied by the libc
unwinder, and is suitable for production variants of Scudo. If Scudo in future
has its own unwinder, it may choose to use its own over the generic unwinder
instead.
Reviewers: cryptoad
Reviewed By: cryptoad
Subscribers: kubamracek, mgorny, #sanitizers, llvm-commits, morehouse, vlad.tsyrklevich, eugenis
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D64085
llvm-svn: 364966
They appear to fail to link in various 32-bit configurations for unknown
reasons. This change was already reverted, and it seems preferable to me
to make forward progress and remove this once the problems are fully
understood.
llvm-svn: 364877
Summary:
This is a redo of D63612.
Two problems came up on some bots:
- `__builtin_umull_overflow` was not declared. This is likely due to an
older clang or gcc, so add a guard with `__has_builtin` and fallback
to a division in the event the builtin doesn't exist;
- contradicting definition for `malloc`, etc. This is AFAIU due to the
fact that we ended up transitively including `stdlib.h` in the `.inc`
due to it being the flags parser header: so move the include to the
cc instead.
This should fix the issues, but since those didn't come up in my local
tests it's mostly guesswork.
Rest is the same!
Reviewers: morehouse, hctim, eugenis, vitalybuka, dyung, hans
Reviewed By: morehouse, dyung, hans
Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63831
llvm-svn: 364547
Makes the build fail with e.g.
llvm/projects/compiler-rt/lib/scudo/standalone/wrappers_c.inc:20:68: error:
declaration of 'void* calloc(size_t, size_t)' has a different exception
specifier
INTERFACE WEAK void *SCUDO_PREFIX(calloc)(size_t nmemb, size_t size) {
^
See llvm-commits thread.
> Summary:
> This CL adds C & C++ wrappers and associated tests. Those use default
> configurations for a Scudo combined allocator that will likely be
> tweaked in the future.
>
> This is the final CL required to have a functional C & C++ allocator
> based on Scudo.
>
> The structure I have chosen is to define the core C allocation
> primitives in an `.inc` file that can be customized through defines.
> This allows to easily have 2 (or more) sets of wrappers backed by
> different combined allocators, as demonstrated by the `Bionic`
> wrappers: one set for the "default" allocator, one set for the "svelte"
> allocator.
>
> Currently all the tests added have been gtests, but I am planning to
> add some more lit tests as well.
>
> Reviewers: morehouse, eugenis, vitalybuka, hctim, rengolin
>
> Reviewed By: morehouse
>
> Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
>
> Tags: #llvm, #sanitizers
>
> Differential Revision: https://reviews.llvm.org/D63612
llvm-svn: 364400
Summary:
There is an error in the shared TSD registry logic when looking for a
TSD in the slow path. There is an unlikely event when a TSD's precedence
was 0 after attempting a `tryLock` which indicated that it was grabbed
by another thread in between. We dealt with that case by continuing to
the next iteration, but that meant that the `Index` was not increased
and we ended up trying to lock the same TSD.
This would manifest in heavy contention, and in the end we would still
lock a TSD, but that was a wasted iteration.
So, do not `continue`, just skip the TSD as a potential candidate.
This is in both the standalone & non-standalone versions.
Reviewers: morehouse, eugenis, vitalybuka, hctim
Reviewed By: morehouse
Subscribers: delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63783
llvm-svn: 364345
Summary:
This CL adds C & C++ wrappers and associated tests. Those use default
configurations for a Scudo combined allocator that will likely be
tweaked in the future.
This is the final CL required to have a functional C & C++ allocator
based on Scudo.
The structure I have chosen is to define the core C allocation
primitives in an `.inc` file that can be customized through defines.
This allows to easily have 2 (or more) sets of wrappers backed by
different combined allocators, as demonstrated by the `Bionic`
wrappers: one set for the "default" allocator, one set for the "svelte"
allocator.
Currently all the tests added have been gtests, but I am planning to
add some more lit tests as well.
Reviewers: morehouse, eugenis, vitalybuka, hctim, rengolin
Reviewed By: morehouse
Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63612
llvm-svn: 364332
Summary:
Fuchsia wants to use mutexes with PI in the Scudo code, as opposed to
our own implementation. This required making `lock` & `unlock` platform
specific (as opposed to `wait` & `wake`) [code courtesy of John
Grossman].
There is an additional flag required now for mappings as well:
`ZX_VM_ALLOW_FAULTS`.
Reviewers: morehouse, mcgrathr, eugenis, vitalybuka, hctim
Reviewed By: morehouse
Subscribers: delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63435
llvm-svn: 363705
Summary:
See D60593 for further information.
This patch adds GWP-ASan support to the Scudo hardened allocator. It also
implements end-to-end integration tests using Scudo as the backing allocator.
The tests include crash handling for buffer over/underflow as well as
use-after-free detection.
Reviewers: vlad.tsyrklevich, cryptoad
Reviewed By: vlad.tsyrklevich, cryptoad
Subscribers: kubamracek, mgorny, #sanitizers, llvm-commits, morehouse
Tags: #sanitizers, #llvm
Differential Revision: https://reviews.llvm.org/D62929
llvm-svn: 363584
Summary:
The Combined allocator hold together all the other components, and
provides a memory allocator interface based on various template
parameters. This will be in turn used by "wrappers" that will provide
the standard C and C++ memory allocation functions, but can be
used as is as well.
This doesn't depart significantly from the current Scudo implementation
except for a few details:
- Quarantine batches are now protected by a header a well;
- an Allocator instance has its own TSD registry, as opposed to a
static one for everybody;
- a function to iterate over busy chunks has been added, for Android
purposes;
This also adds the associated tests, and a few default configurations
for several platforms, that will likely be further tuned later on.
Reviewers: morehouse, hctim, eugenis, vitalybuka
Reviewed By: morehouse
Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63231
llvm-svn: 363569
Summary:
The more tests are added, the more we are limited by the size of the
address space on 32-bit. Implement `unmapTestOnly` all around (like it
is in sanitzer_common) to be able to free up some memory.
This is not intended to be a proper "destructor" for an allocator, but
allows us to not fail due to having no memory left.
Reviewers: morehouse, vitalybuka, eugenis, hctim
Reviewed By: morehouse
Subscribers: delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D63146
llvm-svn: 363095
Summary:
This CL adds the structures dealing with thread specific data for the
allocator. This includes the thread specific data structure itself and
two registries for said structures: an exclusive one, where each thread
will have its own TSD struct, and a shared one, where a pool of TSD
structs will be shared by all threads, with dynamic reassignment at
runtime based on contention.
This departs from the current Scudo implementation: we intend to make
the Registry a template parameter of the allocator (as opposed to a
single global entity), allowing various allocators to coexist with
different TSD registry models. As a result, TSD registry and Allocator
are tightly coupled.
This also corrects a couple of things in other files that I noticed
while adding this.
Reviewers: eugenis, vitalybuka, morehouse, hctim
Reviewed By: morehouse
Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D62258
llvm-svn: 362962
Summary:
This CL introduces the 32 & 64-bit primary allocators, and associated
Local Cache. While the general idea is mostly similar to what exists
in sanitizer_common, it departs from the original code somewhat
significantly:
- the 64-bit primary no longer uses a free array at the end of a region
but uses batches of free blocks in region 0, allowing for a
convergence with the 32-bit primary behavior;
- as a result, there is only one (templated) local cache type for both
primary allocators, and memory reclaiming can be implemented similarly
for the 32-bit & 64-bit platforms;
- 64-bit primary regions are handled a bit differently: we do not
reserve 4TB of memory that we split, but reserve `NumClasses *
2^RegionSizeLog`, each region being offseted by a random number of
pages from its computed base. A side effect of this is that the 64-bit
primary works on 32-bit platform (I don't think we want to encourage
it but it's an interesting side effect);
Reviewers: vitalybuka, eugenis, morehouse, hctim
Reviewed By: morehouse
Subscribers: srhines, mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D61745
llvm-svn: 361159
Summary:
... and its related functions.
The structure and its functionalities are identical to existing ones.
The header stores information on a `scudo::Chunk` to be able to detect
inconsitencies or potential corruption attempts. It is checksummed for
that purpose.
Reviewers: morehouse, eugenis, vitalybuka, hctim
Reviewed By: vitalybuka
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D61654
llvm-svn: 360290
Summary:
The Quarantine is used to hold chunks for a little while prior to
actually releasing them for potential reuse. The code is pretty much
the same as the sanitizer_common one, with additional shuffling of
the quarantine batches to decrease predictability of allocation
patterns when it is enabled.
Reviewers: vitalybuka, eugenis, hctim, morehouse
Reviewed By: morehouse
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D61385
llvm-svn: 360163
Currently VMO in Zircon create using the zx_vmo_create is resizable
by default, but we'll be changing this in the future, requiring an
explicit flag to make the VMO resizable.
Prepare for this change by passing ZX_VMO_RESIZABLE option to all
zx_vmo_create calls that need resizable VMO.
Differential Revision: https://reviews.llvm.org/D61450
llvm-svn: 359803
Summary:
This CL implements the memory reclaiming function `releaseFreeMemoryToOS`
and its associated classes. Most of this code was originally written by
Aleksey for the Primary64 in sanitizer_common, and I made some changes to
be able to implement 32-bit reclaiming as well. The code has be restructured
a bit to accomodate for freelist of batches instead of the freearray used
in the current sanitizer_common code.
Reviewers: eugenis, vitalybuka, morehouse, hctim
Reviewed By: vitalybuka
Subscribers: srhines, mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D61214
llvm-svn: 359567
Summary:
As with the sanitizer_common allocator, the SCM allows for efficient
mapping between sizes and size-classes, table-free.
It doesn't depart significantly from the original, except that we
allow the use of size-class 0 for other purposes (as opposed to
chunks of size 0). The Primary will use it to hold TransferBatches.
Reviewers: vitalybuka, eugenis, hctim, morehouse
Reviewed By: vitalybuka
Subscribers: srhines, mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D61088
llvm-svn: 359199
Summary:
The Secondary allocator wraps the platform allocation primitives. It is
meant to be used for larger sizes that the Primary can't fullfill, as
it will be slower, and sizes are multiple of the system page size.
This also changes some of the existing code, notably the opaque
platform data being passed to the platform specific functions: we can
shave a couple of syscalls on Fuchsia by storing additional data (this
addresses a TODO).
Reviewers: eugenis, vitalybuka, hctim, morehouse
Reviewed By: morehouse
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D60787
llvm-svn: 359097
Summary:
As with other Sanitizers, and the current version of Scudo, we can
provide flags in differents way: at compile time, through a weak
function, through an environment variable.
This change adds support for the configuration flags, and the string
parsers. Those are fairly similar to the sanitizer_common way of doing
things.
Reviewers: morehouse, hctim, vitalybuka
Reviewed By: morehouse, vitalybuka
Subscribers: mgorny, delcypher, jdoerfert, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D59597
llvm-svn: 358011
Summary:
This change adds fatal error messages for various error conditions that
will be added later in the code.
This also addresses a `TODO` now that we have `reportCheckFailed` (which
lead me to notice a few variables that were not cased properly so I
changed that as well).
Reviewers: morehouse, hctim, vitalybuka
Reviewed By: morehouse, hctim, vitalybuka
Subscribers: mgorny, delcypher, jfb, jdoerfert, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D59551
llvm-svn: 356556
Summary:
Add some string utility functions, notably to format strings, get
lengths, convert a string to a number. Those functions will be
used in reports and flags (coming up next). They were mostly borrowed
from sanitizer_common.
Make use of the string length function in a couple places in the
platform code that was checked in with inlined version of it.
Add some tests.
Reviewers: morehouse, eugenis, vitalybuka, hctim
Reviewed By: morehouse, vitalybuka
Subscribers: mgorny, delcypher, jdoerfert, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D59262
llvm-svn: 356457
Summary:
This CL implements the checksumming functions. This departs from the
current Scudo code in one aspect: the software version is no longer
CRC32 but a BSD checksum. This is because the software CRC32 was too
impactful in terms of performances, the BSD checksum has no array
lookup which is better (and saves 1KB of data).
As with the current version, we only flip the CRC compiler flag for
a single compilation unit by default, to allow for a library compiled
with HW CRC32 to work on a system without HW CRC32.
There is some platform & hardware specific code in those files, but
since departs from a mere platform split, it felt right to me to have
it that way.
Reviewers: morehouse, eugenis, vitalybuka, hctim, mcgrathr
Reviewed By: morehouse
Subscribers: mgorny, delcypher, jfb, jdoerfert, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D59116
llvm-svn: 355923
Summary:
This adds simple local & global stats classes to be used by the Primary
and Secondary, and associated test. Note that we don't need the strict
atomicity of the addition & subtraction (as is in sanitizer_common) so
we just use load & store.
Reviewers: morehouse, vitalybuka, eugenis, flowerhack, dmmoore415
Reviewed By: morehouse, vitalybuka
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D59031
llvm-svn: 355643
Summary:
The bytemap classes will be used by the primary32 allocator to associate
classes with memory regions. It's similar to the sanitizer_common one
except for the fact that the base (level1) maps are mapped instead of
being static to reduce the memory footprint of an uninitialized allocator.
Reviewers: vitalybuka, eugenis, morehouse, flowerhack, dmmoore415, mcgrathr
Reviewed By: vitalybuka, morehouse
Subscribers: mgorny, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D58723
llvm-svn: 355416
Summary:
A missing `STATIC` entailed some annoying to debug failures wrt 32 vs 64
binaries. Additionally I noticed I was using the wrong variable (the Scudo
one as opposed to the Scudo Standalone one).
See https://reviews.llvm.org/D58184#1412417 and below for discussion.
Reviewers: vitalybuka, eugenis, brzycki
Reviewed By: vitalybuka, brzycki
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D58794
llvm-svn: 355203
Summary:
This CL adds a standalone vector class that will be used by the scoped
strings when they land. We reimplement our own vector class because we
can't use the std library one.
It's mostly borrowed from the current sanitizer_common one, with LLVM
code style changes.
Additionnally a casing change in a function name that slipped through
the previous review (the function isn't used yet).
Reviewers: vitalybuka, eugenis, flowerhack, dmmoore415, mcgrathr, morehouse
Reviewed By: vitalybuka
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D58689
llvm-svn: 354999
Summary:
This CL adds the platform specific code for Fuchsia, Linux & Android,
as well as some tests related to those (more tests to come later).
While some of it is pretty much a straight port of the existing scudo &
sanitizer_common code, the memory mapping functions have been reworked
a bit to fit the limited usage scenario that Scudo has for them.
For Fuchsia, I can now track the Vmar/Vmo pair for memory mappings if
there is an intent to grow or decommit some mapping (that will be
useful for the Primary).
Reviewers: eugenis, vitalybuka, mcgrathr, phosek, flowerhack, morehouse, dmmoore415
Reviewed By: vitalybuka, morehouse
Subscribers: kcc, dvyukov, srhines, mgorny, delcypher, jfb, jdoerfert, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D58184
llvm-svn: 354895
Add missed value "libcxxabi" and introduce SANITIZER_TEST_CXX for linking
unit tests. This needs to be a full C++ library and cannot be libcxxabi.
Recommit r354132 which I reverted in r354153 because it broke a sanitizer
bot. This was because of the "fixes" for pthread linking, so I've removed
these changes.
Differential Revision: https://reviews.llvm.org/D58012
llvm-svn: 354198
Add missed value "libcxxabi" and introduce SANITIZER_TEST_CXX for linking
unit tests. This needs to be a full C++ library and cannot be libcxxabi.
Differential Revision: https://reviews.llvm.org/D58012
llvm-svn: 354132
Summary:
The standalone Scudo version is being built with `-Werror` which can be
tripped by extraneous command line arguments. We have little control over
those as they can be passed down to us by `CMAKE_C(XX)_FLAGS`, the reported
scenario involving `-stdlib=libc++` (see https://reviews.llvm.org/D57412#1384504).
To work around this, disable `-Wunused-command-line-argument`.
Reviewers: eugenis, vitalybuka, Eugene.Zelenko
Reviewed By: eugenis
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D57757
llvm-svn: 353418
Summary:
This is the initial check-in for the Standalone version of Scudo.
The project is initially going to live in scudo/standalone then will
replace scudo. See http://lists.llvm.org/pipermail/llvm-dev/2019-January/129113.html
for details.
This initial CL is meant to lay out the project structure, of both
code & tests, providing a minimal amount of functionalities, namely
various definitions, some atomic helpers and an intrusive list.
(empty.cc is just here to have a compilation unit, but will go away
in the upcoming CLs).
Initial support is restricted to Linux i386 & x86_64 in make files
and will be extended once things land & work.
We will grow organically from here, adding functionalities in limited
amounts.
Reviewers: morehouse, eugenis, vitalybuka, kcc, mcgrathr, flowerhack
Reviewed By: morehouse, vitalybuka
Subscribers: srhines, mgorny, krytarowski, delcypher, jfb, #sanitizers, llvm-commits
Tags: #llvm, #sanitizers
Differential Revision: https://reviews.llvm.org/D57412
llvm-svn: 353055
Summary:
This tunes several of the default parameters used within the allocator:
- disable the deallocation type mismatch on Android by default; this
was causing too many issues with third party libraries;
- change the default `SizeClassMap` to `Dense`, it caches less entries
and is way more memory efficient overall;
- relax the timing of the RSS checks, 10 times per second was too much,
lower it to 4 times (every 250ms), and update the test so that it
passes with the new default.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: srhines, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D57116
llvm-svn: 352057
to reflect the new license.
We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.
Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.
llvm-svn: 351636
Summary:
This is a follow up patch to r349138.
This patch makes a `AddressSpaceView` a type declaration in the
allocator parameters used by `SizeClassAllocator64`. For ASan, LSan, and
the unit tests the AP64 declarations have been made templated so that
`AddressSpaceView` can be changed at compile time. For the other
sanitizers we just hard-code `LocalAddressSpaceView` because we have no
plans to use these allocators in an out-of-process manner.
rdar://problem/45284065
Reviewers: kcc, dvyukov, vitalybuka, cryptoad, eugenis, kubamracek, george.karpenkov
Subscribers: #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D55764
llvm-svn: 349954
Summary:
This is a follow up patch to r346956 for the `SizeClassAllocator32`
allocator.
This patch makes `AddressSpaceView` a template parameter both to the
`ByteMap` implementations (but makes `LocalAddressSpaceView` the
default), some `AP32` implementations and is used in `SizeClassAllocator32`.
The actual changes to `ByteMap` implementations and
`SizeClassAllocator32` are very simple. However the patch is large
because it requires changing all the `AP32` definitions, and users of
those definitions.
For ASan and LSan we make `AP32` and `ByteMap` templateds type that take
a single `AddressSpaceView` argument. This has been done because we will
instantiate the allocator with a type that isn't `LocalAddressSpaceView`
in the future patches. For the allocators used in the other sanitizers
(i.e. HWAsan, MSan, Scudo, and TSan) use of `LocalAddressSpaceView` is
hard coded because we do not intend to instantiate the allocators with
any other type.
In the cases where untemplated types have become templated on a single
`AddressSpaceView` parameter (e.g. `PrimaryAllocator`) their name has
been changed to have a `ASVT` suffix (Address Space View Type) to
indicate they are templated. The only exception to this are the `AP32`
types due to the desire to keep the type name as short as possible.
In order to check that template is instantiated in the correct a way a
`static_assert(...)` has been added that checks that the
`AddressSpaceView` type used by `Params::ByteMap::AddressSpaceView` matches
the `Params::AddressSpaceView`. This uses the new `sanitizer_type_traits.h`
header.
rdar://problem/45284065
Reviewers: kcc, dvyukov, vitalybuka, cryptoad, eugenis, kubamracek, george.karpenkov
Subscribers: mgorny, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D54904
llvm-svn: 349138
Summary:
This essentially reverts r337010 since it breaks UBSan, which is used
for a few platform libraries. The "-z global" flag is now added for
Scudo as well. The only other sanitizer shared libraries are for asan
and hwasan, which have also been reinstated to use the global flag.
Reviewers: cryptoad, eugenis
Reviewed By: cryptoad
Subscribers: kubamracek, mgorny, delcypher, #sanitizers, nickdesaulniers, chh, kongyi, pirama, llvm-commits
Differential Revision: https://reviews.llvm.org/D52770
llvm-svn: 343599
Summary:
The previous version of the patch makes some code unable to distinguish
failure to map address 0 and error.
Revert to turn the bots back to green while figuring out a new approach.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: kubamracek, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D51451
llvm-svn: 340957
Summary:
The reason for the existence of `eraseHeader` was that it was deemed faster
to null-out a chunk header, effectively making it invalid, rather than marking
it as available, which incurred a checksum computation and a cmpxchg.
A previous use of `eraseHeader` was removed with D50655 due to a race.
Now we remove the second use of it in the Quarantine deallocation path and
replace is with a `compareExchangeHeader`.
The reason for this is that greatly helps debugging some heap bugs as the chunk
header is now valid and the chunk marked available, as opposed to the header
being invalid. Eg: we get an invalid state error, instead of an invalid header
error, which reduces the possibilities. The computational penalty is negligible.
Reviewers: alekseyshl, flowerhack, eugenis
Reviewed By: eugenis
Subscribers: delcypher, jfb, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D51224
llvm-svn: 340633
Summary:
`MmapNoAccess` & `MmapFixedNoAccess` return directly the result of
`internal_mmap`, as opposed to other Mmap functions that return nullptr.
This inconsistency leads to some confusion for the callers, as some check for
`~(uptr)0` (`MAP_FAILED`) for failure (while it can fail with `-ENOMEM` for
example).
Two potential solutions: change the callers, or make the functions return
`nullptr` on failure to follow the precedent set by the other functions.
The second option looked more appropriate to me.
Correct the callers that were wrongly checking for `~(uptr)0` or
`MAP_FAILED`.
TODO for follow up CLs:
- There are a couple of `internal_mmap` calls in XRay that check for
MMAP_FAILED as a result as well (cc: @dberris); they should use
`internal_iserror`;
Reviewers: eugenis, alekseyshl, dberris, kubamracek
Reviewed By: alekseyshl
Subscribers: kristina, kubamracek, delcypher, #sanitizers, dberris, llvm-commits
Differential Revision: https://reviews.llvm.org/D50940
llvm-svn: 340576
Summary:
There is a race window in the deallocation path when the Quarantine is bypassed.
Initially we would just erase the header of a chunk if we were not to use the
Quarantine, as opposed to using a compare-exchange primitive, to make things
faster.
It turned out to be a poor decision, as 2 threads (or more) could simultaneously
deallocate the same pointer, and if the checks were to done before the header
got erased, this would result in the pointer being added twice (or more) to
distinct thread caches, and eventually be reused.
Winning the race is not trivial but can happen with enough control over the
allocation primitives. The repro added attempts to trigger the bug, with a
moderate success rate, but it should be enough to notice if the bug ever make
its way back into the code.
Since I am changing things in this file, there are 2 smaller changes tagging
along, marking a variable `const`, and improving the Quarantine bypass test at
runtime.
Reviewers: alekseyshl, eugenis, kcc, vitalybuka
Reviewed By: eugenis, vitalybuka
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D50655
llvm-svn: 339705
Summary:
There is currently too much redundancy in the class/variable/* names in Scudo:
- we are in the namespace `__scudo`, so there is no point in having something
named `ScudoX` to end up with a final name of `__scudo::ScudoX`;
- there are a lot of types/* that have `Allocator` in the name, given that
Scudo is an allocator I figure this doubles up as well.
So change a bunch of the Scudo names to make them shorter, less redundant, and
overall simpler. They should still be pretty self explaining (or at least it
looks so to me).
The TSD part will be done in another CL (eg `__scudo::ScudoTSD`).
Reviewers: alekseyshl, eugenis
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D49505
llvm-svn: 337557
when building with an IDE so that header files show up in the UI.
This massively improves the development workflow in IDEs.
To implement this a new function `compiler_rt_process_sources(...)` has
been added that adds header files to the list of sources when the
generator is an IDE. For non-IDE generators (e.g. Ninja/Makefile) no
changes are made to the list of source files.
The function can be passed a list of headers via the
`ADDITIONAL_HEADERS` argument. For each runtime library a list of
explicit header files has been added and passed via
`ADDITIONAL_HEADERS`. For `tsan` and `sanitizer_common` a list of
headers was already present but it was stale and has been updated
to reflect the current state of the source tree.
The original version of this patch used file globbing (`*.{h,inc,def}`)
to find the headers but the approach was changed due to this being a
CMake anti-pattern (if the list of headers changes CMake won't
automatically re-generate if globbing is used).
The LLVM repo contains a similar function named `llvm_process_sources()`
but we don't use it here for several reasons:
* It depends on the `LLVM_ENABLE_OPTION` cache variable which is
not set in standalone compiler-rt builds.
* We would have to `include(LLVMProcessSources)` which I'd like to
avoid because it would include a bunch of stuff we don't need.
Differential Revision: https://reviews.llvm.org/D48422
llvm-svn: 336663
Summary:
Namely, set the abort message, and allow to write the message to syslog if the
option is enabled.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D48902
llvm-svn: 336445
Summary:
The C interceptors were using `SIZE_T` defined in the interception library as
a `__sanitizer::uptr`. On some 32-bit platforms, this lead to the following
warning:
```
warning: declaration of ‘void* malloc(SIZE_T)’ conflicts with built-in declaration ‘void* malloc(unsigned int)’ [-Wbuiltin-declaration-mismatch]
INTERCEPTOR_ATTRIBUTE void *malloc(SIZE_T size) {
```
`__sanitizer::uptr` is indeed defined as an `unsigned long` on those.
So just include `stddef.h` and use `size_t` instead.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D48885
llvm-svn: 336221
Summary:
It would be useful for Flutter apps, especially, to be able to use
malloc hooks to debug memory leaks on Fuchsia. They're not able to do
this right now, so it'd be a nice bonus to throw in with the Scudo
switchover.
Reviewers: cryptoad, alekseyshl
Reviewed By: cryptoad
Differential Revision: https://reviews.llvm.org/D48618
llvm-svn: 336139
Summary:
This patch follows D48373.
The point is to be able to use Scudo with `-fsanitize-minimal-runtime`. For that
we need a runtime that doesn't embed the UBSan one. This results in binaries
that can be compiled with `-fsanitize=scudo,integer -fsanitize-minimal-runtime`.
Reviewers: eugenis
Reviewed By: eugenis
Subscribers: mgorny, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D48377
llvm-svn: 335296
Summary:
Mark `isRssLimitExceeded` as `NOINLINE`, and move it's definition as well as
the one of `performSanityChecks` out of the class definition, as requested.
Reviewers: filcab, alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D48228
llvm-svn: 335054
Summary:
The current `FailureHandler` mechanism was fairly opaque with regard to the
failure reason due to using `CHECK(0)`. Scudo is a bit different from the other
Sanitizers as it prefers to avoid spurious processing in its failure path. So
we just `dieWithMessage` using a somewhat explicit string.
Adapted the tests for the new strings.
While this takes care of the `OnBadRequest` & `OnOOM` failures, the next step
is probably to migrate the other Scudo failures in the same failes (header
corruption, invalid state and so on).
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: filcab, mgorny, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D48199
llvm-svn: 334843
Summary:
As a follow up to D48142 for Scudo, switch the `SpinMutex` to its static
counterpart, and ensure zero-initialization by memset'ing the whole class.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D48148
llvm-svn: 334716
Summary:
This CL adds support for aligned new/delete operators (C++17). Currently we
do not support alignment inconsistency detection on deallocation, as this
requires a header change, but the APIs are introduced and are functional.
Add a smoke test for the aligned version of the operators.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D48031
llvm-svn: 334505
Summary:
The shared TSD model in its current form doesn't scale. Here is an example of
rpc2-benchmark (with default parameters, which is threading heavy) on a 72-core
machines (defaulting to a `CompactSizeClassMap` and no Quarantine):
- with tcmalloc: 337K reqs/sec, peak RSS of 338MB;
- with scudo (exclusive): 321K reqs/sec, peak RSS of 637MB;
- with scudo (shared): 241K reqs/sec, peak RSS of 324MB.
This isn't great, since the exclusive model uses a lot of memory, while the
shared model doesn't even come close to be competitive.
This is mostly due to the fact that we are consistently scanning the TSD pool
starting at index 0 for an available TSD, which can result in a lot of failed
lock attempts, and touching some memory that needs not be touched.
This CL attempts to make things better in most situations:
- first, use a thread local variable on Linux (intead of pthread APIs) to store
the current TSD in the shared model;
- move the locking boolean out of the TSD: this allows the compiler to use a
register and potentially optimize out a branch instead of reading it from the
TSD everytime (we also save a tiny bit of memory per TSD);
- 64-bit atomic operations on 32-bit ARM platforms happen to be expensive: so
store the `Precedence` in a `uptr` instead of a `u64`. We lose some
nanoseconds of precision and we'll wrap around at some point, but the benefit
is worth it;
- change a `CHECK` to a `DCHECK`: this should never happen, but if something is
ever terribly wrong, we'll crash on a near null AV if the TSD happens to be
null;
- based on an idea by dvyukov@, we are implementing a bound random scan for
an available TSD. This requires computing the coprimes for the number of TSDs,
and attempting to lock up to 4 TSDs in an random order before falling back to
the current one. This is obviously slightly more expansive when we have just
2 TSDs (barely noticeable) but is otherwise beneficial. The `Precedence` still
basically corresponds to the moment of the first contention on a TSD. To seed
on random choice, we use the precedence of the current TSD since it is very
likely to be non-zero (since we are in the slow path after a failed `tryLock`)
With those modifications, the benchmark yields to:
- with scudo (shared): 330K reqs/sec, peak RSS of 327MB.
So the shared model for this specific situation not only becomes competitive but
outperforms the exclusive model. I experimented with some values greater than 4
for the number of TSDs to attempt to lock and it yielded a decrease in QPS. Just
sticking with the current TSD is also a tad slower. Numbers on platforms with
less cores (eg: Android) remain similar.
Reviewers: alekseyshl, dvyukov, javed.absar
Reviewed By: alekseyshl, dvyukov
Subscribers: srhines, kristof.beyls, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D47289
llvm-svn: 334410
Summary:
It turns out that the previous code construct was not optimizing the allocation
and deallocation of batches. The class id was read as a class member (even
though a precomputed one) and nothing else was optimized. By changing the
construct this way, the compiler actually optimizes most of the allocation and
deallocation away to only work with a single class id, which not only saves some
CPU but also some code footprint.
Reviewers: alekseyshl, dvyukov
Reviewed By: dvyukov
Subscribers: dvyukov, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D46961
llvm-svn: 332502
Summary:
This adds `__scudo_print_stats` as an interface function to display the Primary
and Secondary allocator statistics for Scudo.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D46016
llvm-svn: 330857
Summary:
Fuchsia uses zx_system_get_features in lieu of getauxval.
Use this call when checking for CRC32 support.
Reviewers: cryptoad
Reviewed By: cryptoad
Subscribers: delcypher, llvm-commits, #sanitizers, kristof.beyls, chrib
Differential Revision: https://reviews.llvm.org/D45896
llvm-svn: 330598
Summary:
Host symbolizer & stacktraces related code in their own RT:
`RTSanitizerCommonSymbolizer`, which is "libcdep" by nature. Symbolizer &
stacktraces specific code that used to live in common files is moved to a new
file `sanitizer_symbolizer_report.cc` as is.
The purpose of this is the enforce a separation between code that relies on
symbolization and code that doesn't. This saves the inclusion of spurious code
due to the interface functions with default visibility, and the extra data
associated.
The following sanitizers makefiles were modified & tested locally:
- dfsan: doesn't require the new symbolizer RT
- esan: requires it
- hwasan: requires it
- lsan: requires it
- msan: requires it
- safestack: doesn't require it
- xray: doesn't require it
- tsan: requires it
- ubsan: requires it
- ubsan_minimal: doesn't require it
- scudo: requires it (but not for Fuchsia that has a minimal runtime)
This was tested locally on Linux, Android, Fuchsia.
Reviewers: alekseyshl, eugenis, dberris, kubamracek, vitalybuka, dvyukov, mcgrathr
Reviewed By: alekseyshl, vitalybuka
Subscribers: srhines, kubamracek, mgorny, krytarowski, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D45457
llvm-svn: 330131
Summary:
In the same spirit of SanitizerToolName, allow the Primary & Secondary
allocators to have names that can be set by the tools via PrimaryAllocatorName
and SecondaryAllocatorName.
Additionally, set a non-default name for Scudo.
Reviewers: alekseyshl, vitalybuka
Reviewed By: alekseyshl, vitalybuka
Subscribers: kubamracek, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D45600
llvm-svn: 330055
Summary:
Now that common options are propagated again for runtimes build with D45507,
the -f{data,function}-sections flags are now duplicates, remove them.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D45575
llvm-svn: 329925
Summary:
Fuchsia requires its Scudo shared runtime to not be C++ dependant. Since they
don't use UBSan in conjunction with Scudo, we can just remove the runtime,
and add the extra `nostdinc++` and `nostdlib++` flags. No need for Coverage
either. This allows to keep things going while working on additional splits
of sanitizer_commong and a more minimal runtime.
Reviewers: phosek, flowerhack, alekseyshl
Reviewed By: phosek, alekseyshl
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D44791
llvm-svn: 328625
Summary:
`sanitizer_common`'s coverage support is fairly well separated, and libcdep by
default. Several sanitizers don't make use of coverage, and as far as I can
tell do no benefit from the extra dependencies pulled in by the coverage public
interface functions.
The following sanitizers call `InitializeCoverage` explicitely: MSan, ASan,
LSan, HWAsan, UBSan. On top of this, any sanitizer bundling RTUBSan should
add the coverage RT as well: ASan, Scudo, UBSan, CFI (diag), TSan, MSan, HWAsan.
So in the end the following have no need: DFSan, ESan, CFI, SafeStack (nolibc
anyway), XRay, and the upcoming Scudo minimal runtime.
I tested this with all the sanitizers check-* with gcc & clang, and in
standalone on Linux & Android, and there was no issue. I couldn't test this on
Mac, Fuchsia, BSDs, & Windows for lack of an environment, so adding a bunch of
people for additional scrunity. I couldn't test HWAsan either.
Reviewers: eugenis, vitalybuka, alekseyshl, flowerhack, kubamracek, dberris, rnk, krytarowski
Reviewed By: vitalybuka, alekseyshl, flowerhack, dberris
Subscribers: mgorny, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D44701
llvm-svn: 328204
Summary:
Using `getActuallyAllocatedSize` from the Combined resulting in mediocre
compiled code, as the `ClassId != 0` predicament was not propagated there,
resulting in additional branches and dead code. Move the logic in the frontend,
which results in better compiled code. Also I think it makes it slightly easier
to distinguish between the size the user requested, and the size that was
actually allocated by the allocator.
`const` a couple of things as well.
This has no functional impact.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D44444
llvm-svn: 327525
Summary:
The need for this change stems from the fact that Windows doesn't support
partial unmapping (`MEM_RELEASE` implies the entire allocated region). So we
now have to keep track of the reserved region and the committed region, so that
we can function without the trimming we did when dealing with larger alignments.
Instead of just having a `ReservedAddressRange` per chunk, we introduce a
`LargeChunkHeader` (and `LargeChunk` namespace) that additionally holds the
committed size and the usable size. The former is needed for stats purposes,
the latter is used by the frontend. Requiring both is debatable, we could only
work with the usable size but then be off by up to a page per chunk when
dealing with stats.
Additionally, we introduce more stats since they turned out to be useful for
experiments, and a `PrintStats` function that will be used by the combined
allocator in later patch.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D43949
llvm-svn: 327321
Summary:
A few changes related to logging:
- prepend `Scudo` to the error messages so that users can identify that we
reported an error;
- replace a couple of `Report` calls in the RSS check code with
`dieWithMessage`/`Print`, mark a condition as `UNLIKELY` in the process;
- change some messages so that they all look more or less the same. This
includes the `CHECK` message;
- adapt a couple of tests with the new strings.
A couple of side notes: this results in a few 1-line-blocks, for which I left
brackets. There doesn't seem to be any style guide for that, I can remove them
if need be. I didn't use `SanitizerToolName` in the strings, but directly
`Scudo` because we are the only users, I could change that too.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: mgorny, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D44171
llvm-svn: 326901
Summary:
If not using `-Wl,--gc-sections`, a whole lot of unused `sanitizer_common` code
and related static variables are pulled into the shared library.
Keep the binary size smaller, and its memory footprint smaller as well, by
using the compiler flags `-ffunction-section` & `-fdata-sections` by default,
as well as the linker flags `-Wl,--gc-sections`.
Current experiments show a large discrepency between binary sizes generated
by gcc (big) and clang (small). I am not sure yet how I can make a test that
would encompass both, so it's an outstanding work item.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: mgorny, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D44121
llvm-svn: 326833
Summary:
Instead of using `AlignedChunkHeaderSize`, introduce a `constexpr` function
`getHeaderSize` in the `Chunk` namespace. Switch `RoundUpTo` to a `constexpr`
as well (so we can use it in `constexpr` declarations). Mark a few variables
in the areas touched as `const`.
Overall this has no functional change, and is mostly to make things a bit more
consistent.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D43772
llvm-svn: 326206
Summary:
Allow for options to be defined at compile time, like is already the case for
other sanitizers, via `SCUDO_DEFAULT_OPTIONS`.
Reviewers: alekseyshl, dberris
Reviewed By: alekseyshl, dberris
Subscribers: kubamracek, delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D42980
llvm-svn: 324620
Summary:
Few changes to the secondary:
- mark `const` variables as such;
- change some `CHECK` to `DCHECK`: I don't feel we need to be as conservative as
we were with out checks, as they are the results of our own computation.
- mark a condition as `UNLIKELY`.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42696
llvm-svn: 323997
Summary:
This is in preparation for platforms where `SANITIZER_SUPPORTS_WEAK_HOOKS` is 0.
They require a default implementation.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: delcypher, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D42557
llvm-svn: 323795
Summary:
This is a follow-up to D42506.
There are a few of things that bothered me about `scudo_interceptors.cpp`:
- the filename is a misnomer: it intercepts some functions, but the rest (C++)
is actually in `scudo_new_delete.cpp`. I feel like `scudo_malloc.cpp` is more
appropriate (ASan uses the same naming scheme);
- we do not need "full" interceptors, since we are never accessing the
unsanitized version of the functions, we just need the
`extern "C" INTERCEPTOR_ATTRIBUTE` part of it to just call our functions;
- a couple of functions where duplicated while they could just be `ALIAS`'d;
- use the `SANITIZER_INTERCEPT_*` defines to hide the unneeded interceptors;
- use `SIZE_T` instead of `uptr`: while it's the same behind the curtain,
the former is meant for this use case.
In the end there is no functional change on the currently supported platforms
(Linux, Android).
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: mgorny, hintonda, delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42546
llvm-svn: 323464
Summary:
Currently all platforms are using the `scudo_interceptors.cpp` interceptors.
We might to come up with platform specific interceptors when/if we get Apple &
Windows, but as of now, that allows for Fuchsia to use them.
`scudo_new_delete.cpp` didn't have the `#if SANITIZER_LINUX` so it's good to go.
Reviewers: alekseyshl, flowerhack
Reviewed By: flowerhack
Subscribers: delcypher, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42506
llvm-svn: 323386
Summary:
Hooks in the allocation & deallocation paths can be a security risk (see for an
example https://scarybeastsecurity.blogspot.com/2016/11/0day-exploit-advancing-exploitation.html
which used the glibc's __free_hook to complete exploitation).
But some users have expressed a need for them, even if only for tests and
memory benchmarks. So allow for `__sanitizer_malloc_hook` &
`__sanitizer_free_hook` to be called if defined, and gate them behind a global
define `SCUDO_CAN_USE_HOOKS` defaulting to 0.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42430
llvm-svn: 323278
Summary:
We somehow never did it, and it raised no issue until now, when trying to
enable Fuchsia as a supported Scudo platform in the cmake config.
So propagate SANITIZER_COMMON_LINK_FLAGS for now.
Reviewers: alekseyshl, flowerhack
Reviewed By: flowerhack
Subscribers: mgorny, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42314
llvm-svn: 322999
Summary:
A forgotten include in `scudo_allocator.cpp` made the symbol only local :/
Before:
```
nm ./lib/clang/7.0.0/lib/linux/libclang_rt.scudo-i686-android.so | grep rss
00024730 t __scudo_set_rss_limit
```
After:
```
nm ./lib/clang/7.0.0/lib/linux/libclang_rt.scudo-i686-android.so | grep rs
00024760 T __scudo_set_rss_limit
```
And we want `T`!
This include also means that we can get rid of the `extern "C"` in the C++
file, the compiler does fine without it (note that this was already the case
for all the `__sanitizer_*` interface functions.
Reviewers: alekseyshl, eugenis
Reviewed By: eugenis
Subscribers: #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42199
llvm-svn: 322782
Summary:
jemalloc on Android currently uses 2 arenas
(https://android.googlesource.com/platform/external/jemalloc/+/master/Android.bp#64).
Since the Android toolchain absorbs compiler-rt and compiles it as is, we have
to enforce the same limit to somehow stay competitive in terms of memory usage.
The changes could either go in:
- `scudo_platform.h` with a default for Android of 2 (this is the solution
implemented here);
- in `CMakeLists.txt` adding -DSCUDO_SHARED_TSD_POOL_SIZE=2 for Android.
- something else?
I don't have a strong opinion on how to do it, but it has to be done upstream
anyway.
Reviewers: alekseyshl, eugenis
Reviewed By: alekseyshl, eugenis
Subscribers: srhines, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D42194
llvm-svn: 322764
Summary:
This is needed for the shared runtime since we are pulling RTUbsan in.
Otherwise some builds might fail with errors such as:
`error: undefined reference to '__dynamic_cast'`
Reviewers: alekseyshl, srhines
Reviewed By: srhines
Subscribers: kongyi, pirama, chh, mgorny, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D41995
llvm-svn: 322389
Summary:
`__scudo_set_rss_limit`'s `LimitMb` should really be a `size_t`. Update
accordingly the prototype. To avoid the `NOLINT` and conform with the other
Sanitizers, use the sanitizers types for the internal definition. This should
have no functional change.
Additionally, capitalize a variable name to follow the LLVM coding standards.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D41704
llvm-svn: 321803
Summary:
The initial implementation used an ASan like Chunk class that was deriving from
a Header class. Due to potential races, we ended up working with local copies
of the Header and never using the parent class fields. ScudoChunk was never
constructed but cast, and we were using `this` as the pointer needed for our
computations. This was meh.
So we refactored ScudoChunk to be now a series of static functions within the
namespace `__scudo::Chunk` that take a "user" pointer as first parameter (former
`this`). A compiled binary doesn't really change, but the code is more sensible.
Clang tends to inline all those small function (in -O2), but GCC left a few not
inlined, so we add the `INLINE` keyword to all.
Since we don't have `ScudoChunk` pointers anymore, a few variables were renamed
here and there to introduce a clearer distinction between a user pointer
(usually `Ptr`) and a backend pointer (`BackendPtr`).
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D41200
llvm-svn: 320745
Summary:
The first and only function to start with allows to set the soft or hard RSS
limit at runtime. Add associated tests.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: mgorny, #sanitizers, llvm-commits
Differential Revision: https://reviews.llvm.org/D41128
llvm-svn: 320611
Summary:
See D40657 & D40679 for previous versions of this patch & description.
A couple of things were fixed here to have it not break some bots.
Weak symbols can't be used with `SANITIZER_GO` so the previous version was
breakin TsanGo. I set up some additional local tests and those pass now.
I changed the workaround for the glibc vDSO issue: `__progname` is initialized
after the vDSO and is actually public and of known type, unlike
`__vdso_clock_gettime`. This works better, and with all compilers.
The rest is the same.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, kubamracek, krytarowski, llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D41121
llvm-svn: 320594
Summary:
getScudoChunk function is implicitly inlined for optimized builds on
clang, but not on gcc. It's a small enough function that it seems
sensible enough to just inline it by default.
Reviewers: cryptoad, alekseyshl
Reviewed By: cryptoad
Differential Revision: https://reviews.llvm.org/D41138
llvm-svn: 320592
Summary:
Redo of D40657, which had the initial discussion. The initial code had to move
into a libcdep file, and things had to be shuffled accordingly.
`NanoTime` is a time sink when checking whether or not to release memory to
the OS. While reducing the amount of calls to said function is in the works,
another solution that was found to be beneficial was to use a timing function
that can leverage the vDSO.
We hit a couple of snags along the way, like the fact that the glibc crashes
when clock_gettime is called from a preinit_array, or the fact that
`__vdso_clock_gettime` is mangled (for security purposes) and can't be used
directly, and also that clock_gettime can be intercepted.
The proposed solution takes care of all this as far as I can tell, and
significantly improve performances and some Scudo load tests with memory
reclaiming enabled.
@mcgrathr: please feel free to follow up on
https://reviews.llvm.org/D40657#940857 here. I posted a reply at
https://reviews.llvm.org/D40657#940974.
Reviewers: alekseyshl, krytarowski, flowerhack, mcgrathr, kubamracek
Reviewed By: alekseyshl, krytarowski
Subscribers: #sanitizers, mcgrathr, srhines, llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D40679
llvm-svn: 320409
Summary:
It looks like clang was generating somewhat weird assembly with the current
code. `FromPrimary`, even though `const`, was replaced every time with the code
generated for `size <= SizeClassMap::kMaxSize` instead of using a variable or
register, and `FromPrimary` didn't induce `ClassId != 0` for the compiler, so a
dead branch was generated for `getActuallyAllocatedSize(Ptr, ClassId)` since
it's never called for `ClassId = 0` (Secondary backed allocations) [this one
was more wishful thinking on my side than anything else].
I rearranged the code bit so that the generated assembly is less clunky.
Also changed 2 whitespace inconsistencies that were bothering me.
Reviewers: alekseyshl, flowerhack
Reviewed By: flowerhack
Subscribers: llvm-commits, #sanitizers
Differential Revision: https://reviews.llvm.org/D40976
llvm-svn: 320160
Summary:
This wasn't noticed: `RoundUpTo` doesn't produce a constant expression, so the
sizes were not constant either. Enforce them to be static const, replace
`RoundUpTo` by its expression. The compiler can now optimize the associated
computations accordingly.
Also looking at the produced assembly, `PageSize` was fetched multiple times
during `Allocate`, so keep a local value of it. As a result it's fetched once
and kept in a register.
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D40862
llvm-svn: 319903
Summary:
It was deemed that the salt in the chunk header didn't improve security
significantly (and could actually decrease it). The initial idea was that the
same chunk would different headers on different allocations, allowing for less
predictability. The issue is that gathering the same chunk header with different
salts can give information about the other "secrets" (cookie, pointer), and that
if an attacker leaks a header, they can reuse it anyway for that same chunk
anyway since we don't enforce the salt value.
So we get rid of the salt in the header. This means we also get rid of the
thread local Prng, and that we don't need a global Prng anymore as well. This
makes everything faster.
We reuse those 8 bits to store the `ClassId` of a chunk now (0 for a secondary
based allocation). This way, we get some additional speed gains:
- `ClassId` is computed outside of the locked block;
- `getActuallyAllocatedSize` doesn't need the `GetSizeClass` call;
- same for `deallocatePrimary`;
We add a sanity check at init for this new field (all sanity checks are moved
in their own function, `init` was getting crowded).
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D40796
llvm-svn: 319791
Summary:
With this change, we allow someone to chose the `SizeClassMap` they want to use
at compile time via a define.
I feel somewhat unimaginative with the name of the defines, so if someone has a
better idea, let me know. I have been alternating between those and
`SCUDO_USE_xxx_SIZECLASSMAP` which is clearer but also longer. The issue with
those is that it wouldn't be consistent with `SCUDO_TSD_EXCLUSIVE` that should
probably become `SCUDO_USE_EXCLUSIVE_TSD` maybe?
Anyway, naming is hard, and I am not sure what makes more sense!
Reviewers: alekseyshl, flowerhack
Reviewed By: alekseyshl
Subscribers: llvm-commits, srhines
Differential Revision: https://reviews.llvm.org/D40521
llvm-svn: 319350
Summary:
Bionic doesn't initialize its globals early enough. This causes issues when
trying to access them from a preinit_array (b/25751302) or from another
constructor called before the libc one (b/68046352). __progname is initialized
after the other globals, so we can check its value to know if calling
`getauxval` is safe.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, llvm-commits
Differential Revision: https://reviews.llvm.org/D40504
llvm-svn: 319099
Summary:
This patch aims at condensing the hardware CRC32 feature detection and making
it slightly more effective on Android.
The following changes are included:
- remove the `CPUFeature` enum, and get rid of one level of nesting of
functions: we only used CRC32, so we just implement and use
`hasHardwareCRC32`;
- allow for a weak `getauxval`: the Android toolchain is compiled at API level
14 for Android ARM, meaning no `getauxval` at compile time, yet we will run
on API level 27+ devices. The `/proc/self/auxv` fallback can work but is
worthless for a process like `init` where the proc filesystem doesn't exist
yet. If a weak `getauxval` doesn't exist, then fallback.
- couple of extra corrections.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: kubamracek, aemerson, srhines, kristof.beyls, llvm-commits
Differential Revision: https://reviews.llvm.org/D40322
llvm-svn: 318859
Summary:
This change allows Fuchsia to boot properly using the Scudo allocator.
A first version of this commit was reverted by rL317834 because it broke Android
builds for toolchains generated with older NDKs. This commit introduces a
fall back to solve that issue.
Reviewers: cryptoad, krytarowski, rnk, alekseyshl
Reviewed By: cryptoad, krytarowski, alekseyshl
Subscribers: llvm-commits, srhines, kubamracek, krytarowski
Differential Revision: https://reviews.llvm.org/D40121
llvm-svn: 318802
Summary:
This implements an opportunistic check for the RSS limit.
For ASan, this was implemented thanks to a background thread checking the
current RSS vs the set limit every 100ms. This was deemed problematic for Scudo
due to potential Android concerns (Zygote as pointed out by Aleksey) as well as
the general inconvenience of having a permanent background thread.
If a limit (soft or hard) is specified, we will attempt to update the RSS limit
status (exceeded or not) every 100ms. This is done in an opportunistic way: if
we can update it, we do it, if not we return the current status, mostly because
we don't need it to be fully consistent (it's done every 100ms anyway). If the
limit is exceeded `allocate` will act as if OOM for a soft limit, or just die
for a hard limit.
We use the `common_flags()`'s `hard_rss_limit_mb` & `soft_rss_limit_mb` for
configuration of the limits.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D40038
llvm-svn: 318301
Summary:
This is mostly some cleanup and shouldn't affect functionalities.
Reviewing some code for a future addition, I realized that the complexity of
the initialization path was unnecessary, and so was maintaining a structure
for the allocator options throughout the initialization.
So we get rid of that structure, of an extraneous level of nesting for the
`init` function, and correct a couple of related code inaccuracies in the
flags cpp.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D39974
llvm-svn: 318157
Summary:
The ScudoAllocator uses a SecondaryHeader to keep track of the size and base address of each mmap'd chunk.
This aligns well with what the ReservedAddressRange is trying to do. This changeset converts the scudo allocator from using the MmapNoAccess/MmapFixed APIs to the ReservedAddressRange::Init and ::Map APIs. In doing so, it replaces the SecondayHeader struct with the ReservedAddressRange object.
This is part 3 of a 4 part changeset; part 1 https://reviews.llvm.org/D39072 and part 2 https://reviews.llvm.org/D38592
Reviewers: alekseyshl, mcgrathr, cryptoad, phosek
Reviewed By: cryptoad
Subscribers: llvm-commits, cryptoad, kubamracek
Differential Revision: https://reviews.llvm.org/D38593
llvm-svn: 318080
Summary:
`getauxval` was introduced in 18 & 21 depending on the architecture. Bump the
requirement to 21.
It also turns out that the NDK is finicky: NDK r13b doesn't include sys/auxv.h
when creating a standalone toolchain at API level 19 for ARM. So 18 didn't work
well with older NDKs.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: aemerson, srhines, llvm-commits, kristof.beyls
Differential Revision: https://reviews.llvm.org/D39905
llvm-svn: 317907
Summary:
This reverts D39490.
For toolchains generated with older NDKs (<=r13b as far as we tested),
`cpu_set_t` doesn't exist in `sched.h`.
We have to figure out another way to get the number of CPUs without this.
Reviewers: rnk
Reviewed By: rnk
Subscribers: kubamracek, llvm-commits, krytarowski
Differential Revision: https://reviews.llvm.org/D39867
llvm-svn: 317834
Summary:
Scudo abides by the coding style enforced by the sanitizer_common
linter, but as of right now, it's not linter-enforced.
Add Scudo to the list of directories checked by check_lint.sh.
Also: fixes some linter errors found after getting this running.
Reviewers: cryptoad
Reviewed By: cryptoad
Subscribers: llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D39757
llvm-svn: 317699
Summary:
Initially, Scudo had a monolithic design where both C and C++ functions were
living in the same library. This was not necessarily ideal, and with the work
on -fsanitize=scudo, it became more apparent that this needed to change.
We are splitting the new/delete interceptor in their own C++ library. This
allows more flexibility, notably with regard to std::bad_alloc when the work is
done. This also allows us to not link new & delete when using pure C.
Additionally, we add the UBSan runtimes with Scudo, in order to be able to have
a -fsanitize=scudo,undefined in Clang (see work in D39334).
The changes in this patch:
- split the cxx specific code in the scudo cmake file into a new library;
(remove the spurious foreach loop, that was not necessary)
- add the UBSan runtimes (both C and C++);
- change the test cmake file to allow for specific C & C++ tests;
- make C tests pure C, rename their extension accordingly.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, mgorny, llvm-commits
Differential Revision: https://reviews.llvm.org/D39461
llvm-svn: 317097
Summary:
This introduces `SCUDO_MAX_CACHES` allowing to define an upper bound to the
number of `ScudoTSD` created in the Shared TSD model (by default 32U).
This name felt clearer than `SCUDO_MAX_TSDS` which is technically what it really
is. I am opened to suggestions if that doesn't feel right.
Additionally change `getNumberOfCPUs` to return a `u32` to be more consistent.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D39338
llvm-svn: 316788
Summary:
The 32-bit allocator is now on par with the 64-bit in terms of security (chunks
randomization is done, batches separation is done).
Unless objection, the comment can go away.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D39303
llvm-svn: 316620
Summary:
Up to now, the Scudo cmake target only provided a static library that had to be
linked to an executable to benefit from the hardened allocator.
This introduces a shared library as well, that can be LD_PRELOAD'ed.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, mgorny, llvm-commits
Differential Revision: https://reviews.llvm.org/D38980
llvm-svn: 316342
Summary:
Move the `sanitizer_posix.h` include within the `SANITIZER_ANDROID` `#if`,
otherwise this errors when built on non-Posix platforms (eg: Fuchsia).
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D38956
llvm-svn: 315917
Summary:
Follow up to D38826.
We introduce `pthread_{get,set}specific` versions of `{get,set}CurrentTSD` to
allow for non Android platforms to use the Shared TSD model.
We now allow `SCUDO_TSD_EXCLUSIVE` to be defined at compile time.
A couple of things:
- I know that `#if SANITIZER_ANDROID` is not ideal within a function, but in
the end I feel it looks more compact and clean than going the .inc route; I
am open to an alternative if anyone has one;
- `SCUDO_TSD_EXCLUSIVE=1` requires ELF TLS support (and not emutls as this uses
malloc). I haven't found anything to enforce that, so it's currently not
checked.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, llvm-commits
Differential Revision: https://reviews.llvm.org/D38854
llvm-svn: 315751
Summary:
This first part just prepares the grounds for part 2 and doesn't add any new
functionality. It mostly consists of small refactors:
- move the `pthread.h` include higher as it will be used in the headers;
- use `errno.h` in `scudo_allocator.cpp` instead of the sanitizer one, update
the `errno` assignments accordingly (otherwise it creates conflicts on some
platforms due to `pthread.h` including `errno.h`);
- introduce and use `getCurrentTSD` and `setCurrentTSD` for the shared TSD
model code;
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits, srhines
Differential Revision: https://reviews.llvm.org/D38826
llvm-svn: 315583
Summary:
Previous parts: D38139, D38183.
In this part of the refactor, we abstract the Linux vs Android TSD dissociation
in favor of a Exclusive vs Shared one, allowing for easier platform introduction
and configuration.
Most of this change consist of shuffling the files around to reflect the new
organization.
We introduce `scudo_platform.h` where platform specific definition lie. This
involves the TSD model and the platform specific allocator parameters. In an
upcoming CL, those will be configurable via defines, but we currently stick
with conservative defaults.
Reviewers: alekseyshl, dvyukov
Reviewed By: alekseyshl, dvyukov
Subscribers: srhines, llvm-commits, mgorny
Differential Revision: https://reviews.llvm.org/D38244
llvm-svn: 314224
Summary:
Following D38139, we now consolidate the TSD definition, merging the shared
TSD definition with the exclusive TSD definition. We introduce a boolean set
at initializaton denoting the need for the TSD to be unlocked or not. This
adds some unused members to the exclusive TSD, but increases consistency and
reduces the definitions fragmentation.
We remove the fallback mechanism from `scudo_allocator.cpp` and add a fallback
TSD in the non-shared version. Since the shared version doesn't require one,
this makes overall more sense.
There are a couple of additional cosmetic changes: removing the header guards
from the remaining `.inc` files, added error string to a `CHECK`.
Question to reviewers: I thought about friending `getTSDAndLock` in `ScudoTSD`
so that the `FallbackTSD` could `Mutex.Lock()` directly instead of `lock()`
which involved zeroing out the `Precedence`, which is unused otherwise. Is it
worth doing?
Reviewers: alekseyshl, dvyukov, kcc
Reviewed By: dvyukov
Subscribers: srhines, llvm-commits
Differential Revision: https://reviews.llvm.org/D38183
llvm-svn: 314110
Summary:
We are going through an overhaul of Scudo's TSD, to allow for new platforms
to be integrated more easily, and make the code more sound.
This first part is mostly renaming, preferring some shorter names, correcting
some comments. I removed `getPrng` and `getAllocatorCache` to directly access
the members, there was not really any benefit to them (and it was suggested by
Dmitry in D37590).
The only functional change is in `scudo_tls_android.cpp`: we enforce bounds to
the `NumberOfTSDs` and most of the logic in `getTSDAndLockSlow` is skipped if we
only have 1 TSD.
Reviewers: alekseyshl, dvyukov, kcc
Reviewed By: dvyukov
Subscribers: llvm-commits, srhines
Differential Revision: https://reviews.llvm.org/D38139
llvm-svn: 313987
Summary:
In a few functions (`scudoMemalign` and the like), we would call
`ScudoAllocator::FailureHandler::OnBadRequest` if the parameters didn't check
out. The issue is that if the allocator had not been initialized (eg: if this
is the first heap related function called), we would use variables like
`allocator_may_return_null` and `exitcode` that still had their default value
(as opposed to the one set by the user or the initialization path).
To solve this, we introduce `handleBadRequest` that will call `initThreadMaybe`,
allowing the options to be correctly initialized.
Unfortunately, the tests were passing because `exitcode` was still 0, so the
results looked like success. Change those tests to do what they were supposed
to.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D37853
llvm-svn: 313294
Summary:
Some of glibc's own thread local data is destroyed after a user's thread local
destructors are called, via __libc_thread_freeres. This might involve calling
free, as is the case for strerror_thread_freeres.
If there is no prior heap operation in the thread, this free would end up
initializing some thread specific data that would never be destroyed properly
(as user's pthread destructors have already been called), while still being
deallocated when the TLS goes away. As a result, a program could SEGV, usually
in __sanitizer::AllocatorGlobalStats::Unregister, where one of the doubly linked
list links would refer to a now unmapped memory area.
To prevent this from happening, we will not do a full initialization from the
deallocation path. This means that the fallback cache & quarantine will be used
if no other heap operation has been called, and we effectively prevent the TSD
being initialized and never destroyed. The TSD will be fully initialized for all
other paths.
In the event of a thread doing only frees and nothing else, a TSD would never
be initialized for that thread, but this situation is unlikely and we can live
with that.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D37697
llvm-svn: 312939
Summary:
`getauxval` was introduced with API level 18. In order to get things to work
at lower API levels (for the toolchain itself which is built at 14 for 32-bit),
we introduce an alternative implementation reading directly from
`/proc/self/auxv`.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, llvm-commits
Differential Revision: https://reviews.llvm.org/D37488
llvm-svn: 312653
Summary:
Currently `TransferBatch` are located within the same memory regions as
"regular" chunks. This is not ideal for security: they make for an interesting
target to overwrite, and are not protected by the frontend (namely, Scudo).
To solve this, we re-introduce `kUseSeparateSizeClassForBatch` for the 32-bit
Primary allowing for `TransferBatch` to end up in their own memory region.
Currently only Scudo would use this new feature, the default behavior remains
unchanged. The separate `kBatchClassID` was used for a brief period of time
previously but removed when the 64-bit ended up using the "free array".
Reviewers: alekseyshl, kcc, eugenis
Reviewed By: alekseyshl
Subscribers: llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D37082
llvm-svn: 311891
Summary:
This patch changes a few (small) things around for compatibility purposes for
the current Android & Fuchsia work:
- `realloc`'ing some memory that was not allocated with `malloc`, `calloc` or
`realloc`, while UB according to http://pubs.opengroup.org/onlinepubs/009695399/functions/realloc.html
is more common that one would think. We now only check this if
`DeallocationTypeMismatch` is set; change the "mismatch" error
messages to be more homogeneous;
- some sketchily written but widely used libraries expect a call to `realloc`
to copy the usable size of the old chunk to the new one instead of the
requested size. We have to begrundingly abide by this de-facto standard.
This doesn't seem to impact security either way, unless someone comes up with
something we didn't think about;
- the CRC32 intrinsics for 64-bit take a 64-bit first argument. This is
misleading as the upper 32 bits end up being ignored. This was also raising
`-Wconversion` errors. Change things to take a `u32` as first argument.
This also means we were (and are) only using 32 bits of the Cookie - not a
big thing, but worth mentioning.
- Includes-wise: prefer `stddef.h` to `cstddef`, move `scudo_flags.h` where it
is actually needed.
- Add tests for the memalign-realloc case, and the realloc-usable-size one.
(Edited typos)
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D36754
llvm-svn: 311018
Summary:
On platforms with `getrandom`, the system call defaults to blocking. This
becomes an issue in the very early stage of the boot for Scudo, when the RNG
source is not set-up yet: the syscall will block and we'll stall.
Introduce a parameter to specify that the function should not block, defaulting
to blocking as the underlying syscall does.
Update Scudo to use the non-blocking version.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D36399
llvm-svn: 310839
Summary:
Previously we were rounding up the size passed to `pvalloc` to the next
multiple of page size no matter what. There is an overflow possibility that
wasn't accounted for. So now, return null in the event of an overflow. The man
page doesn't seem to indicate the errno to set in this particular situation,
but the glibc unit tests go for ENOMEM (https://code.woboq.org/userspace/glibc/malloc/tst-pvalloc.c.html#54)
so we'll do the same.
Update the aligned allocation funtions tests to check for properly aligned
returned pointers, and the `pvalloc` corner cases.
@alekseyshl: do you want me to do the same in the other Sanitizers?
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: kubamracek, alekseyshl, llvm-commits
Differential Revision: https://reviews.llvm.org/D35818
llvm-svn: 309033
Summary:
First, some context.
The main feedback we get about the quarantine is that it's too memory hungry.
A single MB of quarantine will have an impact of 3 to 4MB of PSS/RSS, and
things quickly get out of hand in terms of memory usage, and the quarantine
ends up disabled.
The main objective of the quarantine is to protect from use-after-free
exploitation by making it harder for an attacker to reallocate a controlled
chunk in place of the targeted freed chunk. This is achieved by not making it
available to the backend right away for reuse, but holding it a little while.
Historically, what has usually been the target of such attacks was objects,
where vtable pointers or other function pointers could constitute a valuable
targeti to replace. Those are usually on the smaller side. There is barely any
advantage in putting the quarantine several megabytes of RGB data or the like.
Now for the patch.
This patch introduces a new way the Quarantine behaves in Scudo. First of all,
the size of the Quarantine will be defined in KB instead of MB, then we
introduce a new option: the size up to which (lower than or equal to) a chunk
will be quarantined. This way, we only quarantine smaller chunks, and the size
of the quarantine remains manageable. It also prevents someone from triggering
a recycle by allocating something huge. We default to 512 bytes on 32-bit and
2048 bytes on 64-bit platforms.
In details, the patches includes the following:
- introduce `QuarantineSizeKb`, but honor `QuarantineSizeMb` if set to fall
back to the old behavior (meaning no threshold in that case);
`QuarantineSizeMb` is described as deprecated in the options descriptios;
documentation update will follow;
- introduce `QuarantineChunksUpToSize`, the new threshold value;
- update the `quarantine.cpp` test, and other tests using `QuarantineSizeMb`;
- remove `AllocatorOptions::copyTo`, it wasn't used;
- slightly change the logic around `quarantineOrDeallocateChunk` to accomodate
for the new logic; rename a couple of variables there as well;
Rewriting the tests, I found a somewhat annoying bug where non-default aligned
chunks would account for more than needed when placed in the quarantine due to
`<< MinAlignment` instead of `<< MinAlignmentLog`. This is fixed and tested for
now.
Reviewers: alekseyshl, kcc
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D35694
llvm-svn: 308884
Summary:
ASan/MSan/LSan allocators set errno on allocation failures according to
malloc/calloc/etc. expected behavior.
MSan allocator was refactored a bit to make its structure more similar
with other allocators.
Also switch Scudo allocator to the internal errno definitions.
TSan allocator changes will follow.
Reviewers: eugenis
Subscribers: llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D35275
llvm-svn: 308344
Summary:
Set proper errno code on alloction failure and change pvalloc and
posix_memalign implementation to satisfy their man-specified
requirements.
Reviewers: cryptoad
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D35429
llvm-svn: 308053
Summary:
Secondary backed allocations do not require a cache. While it's not necessary
an issue when each thread has its cache, it becomes one with a shared pool of
caches (Android), as a Secondary backed allocation or deallocation holds a
cache that could be useful to another thread doing a Primary backed allocation.
We introduce an additional PRNG and its mutex (to avoid contention with the
Fallback one for Primary allocations) that will provide the `Salt` needed for
Secondary backed allocations.
I changed some of the code in a way that feels more readable to me (eg: using
some values directly rather than going through ternary assigned variables,
using directly `true`/`false` rather than `FromPrimary`). I will let reviewers
decide if it actually is.
An additional change is to mark `CheckForCallocOverflow` as `UNLIKELY`.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D35358
llvm-svn: 307958
Summary:
This follows the addition of `GetRandom` with D34412. We remove our
`/dev/urandom` code and use the new function. Additionally, change the PRNG for
a slightly faster version. One of the issues with the old code is that we have
64 full bits of randomness per "next", using only 8 of those for the Salt and
discarding the rest. So we add a cached u64 in the PRNG that can serve up to
8 u8 before having to call the "next" function again.
During some integration work, I also realized that some very early processes
(like `init`) do not benefit from `/dev/urandom` yet. So if there is no
`getrandom` syscall as well, we have to fallback to some sort of initialization
of the PRNG.
Now a few words on why XoRoShiRo and not something else. I have played a while
with various PRNGs on 32 & 64 bit platforms. Some results are below. LCG 32 & 64
are usually faster but produce respectively 15 & 31 bits of entropy, meaning
that to get a full 64-bit, you would need to call them several times. The simple
XorShift is fast, produces 32 bits but is mediocre with regard to PRNG test
suites, PCG is slower overall, and XoRoShiRo is faster than XorShift128+ and
produces full 64 bits.
%%%
root@tulip-chiphd:/data # ./randtest.arm
[+] starting xs32...
[?] xs32 duration: 22431833053ns
[+] starting lcg32...
[?] lcg32 duration: 14941402090ns
[+] starting pcg32...
[?] pcg32 duration: 44941973771ns
[+] starting xs128p...
[?] xs128p duration: 48889786981ns
[+] starting lcg64...
[?] lcg64 duration: 33831042391ns
[+] starting xos128p...
[?] xos128p duration: 44850878605ns
root@tulip-chiphd:/data # ./randtest.aarch64
[+] starting xs32...
[?] xs32 duration: 22425151678ns
[+] starting lcg32...
[?] lcg32 duration: 14954255257ns
[+] starting pcg32...
[?] pcg32 duration: 37346265726ns
[+] starting xs128p...
[?] xs128p duration: 22523807219ns
[+] starting lcg64...
[?] lcg64 duration: 26141304679ns
[+] starting xos128p...
[?] xos128p duration: 14937033215ns
%%%
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: aemerson, kristof.beyls, llvm-commits
Differential Revision: https://reviews.llvm.org/D35221
llvm-svn: 307798
Summary:
We were not following the `man` documented behaviors for invalid arguments to
`memalign` and associated functions. Using `CHECK` for those was a bit extreme,
so we relax the behavior to return null pointers as expected when this happens.
Adapt the associated test.
I am using this change also to change a few more minor performance improvements:
- mark as `UNLIKELY` a bunch of unlikely conditions;
- the current `CHECK` in `__sanitizer::RoundUpTo` is redundant for us in *all*
calls. So I am introducing our own version without said `CHECK`.
- change our combined allocator `GetActuallyAllocatedSize`. We already know if
the pointer is from the Primary or Secondary, so the `PointerIsMine` check is
redundant as well, and costly for the 32-bit Primary. So we get the size by
directly using the available Primary functions.
Finally, change a `int` to `uptr` to avoid a warning/error when compiling on
Android.
Reviewers: alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D34782
llvm-svn: 306698
Summary:
Operator new interceptors behavior is now controlled by their nothrow
property as well as by allocator_may_return_null flag value:
- allocator_may_return_null=* + new() - die on allocation error
- allocator_may_return_null=0 + new(nothrow) - die on allocation error
- allocator_may_return_null=1 + new(nothrow) - return null
Ideally new() should throw std::bad_alloc exception, but that is not
trivial to achieve, hence TODO.
Reviewers: eugenis
Subscribers: kubamracek, llvm-commits
Differential Revision: https://reviews.llvm.org/D34731
llvm-svn: 306604
Summary:
Move cached allocator_may_return_null flag to sanitizer_allocator.cc and
provide API to consolidate and unify the behavior of all specific allocators.
Make all sanitizers using CombinedAllocator to follow
AllocatorReturnNullOrDieOnOOM() rules to behave the same way when OOM
happens.
When OOM happens, turn allocator_out_of_memory flag on regardless of
allocator_may_return_null flag value (it used to not to be set when
allocator_may_return_null == true).
release_to_os_interval_ms and rss_limit_exceeded will likely be moved to
sanitizer_allocator.cc too (later).
Reviewers: eugenis
Subscribers: srhines, kubamracek, llvm-commits
Differential Revision: https://reviews.llvm.org/D34310
llvm-svn: 305858
Summary:
Currently we are not enforcing the success of `pthread_once`, and
`pthread_setspecific`. Errors could lead to harder to debug issues later in
the thread's life. This adds checks for a 0 return value for both.
If `pthread_setspecific` fails in the teardown path, opt for an immediate
teardown as opposed to a fatal failure.
Reviewers: alekseyshl, kcc
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D33555
llvm-svn: 303998
Summary:
After discussing the current defaults with a couple of parties, the consensus
is that they are too high. 1Mb of quarantine has about a 4Mb impact on PSS, so
memory usage goes up quickly.
This is obviously configurable, but the default value should be more
"approachable", so both the global size and the thread local size are 1/4 of
what they used to be.
Reviewers: alekseyshl, kcc
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D33321
llvm-svn: 303380
Summary:
With rL279771, SizeClassAllocator64 was changed to accept only one template
instead of 5, for the following reasons: "First, this will make the mangled
names shorter. Second, this will make adding more parameters simpler". This
patch mirrors that work for SizeClassAllocator32.
This is in preparation for introducing the randomization of chunks in the
32-bit SizeClassAllocator in a later patch.
Reviewers: kcc, alekseyshl, dvyukov
Reviewed By: alekseyshl
Subscribers: llvm-commits, kubamracek
Differential Revision: https://reviews.llvm.org/D33141
llvm-svn: 303071
Summary:
The reasoning behind this change is twofold:
- the current combined allocator (sanitizer_allocator_combined.h) implements
features that are not relevant for Scudo, making some code redundant, and
some restrictions not pertinent (alignments for example). This forced us to
do some weird things between the frontend and our secondary to make things
work;
- we have enough information to be able to know if a chunk will be serviced by
the Primary or Secondary, allowing us to avoid extraneous calls to functions
such as `PointerIsMine` or `CanAllocate`.
As a result, the new scudo-specific combined allocator is very straightforward,
and allows us to remove some now unnecessary code both in the frontend and the
secondary. Unused functions have been left in as unimplemented for now.
It turns out to also be a sizeable performance gain (3% faster in some Android
memory_replay benchmarks, doing some more on other platforms).
Reviewers: alekseyshl, kcc, dvyukov
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D33007
llvm-svn: 302830
Summary:
This change optimizes several aspects of the checksum used for chunk headers.
First, there is no point in checking the weak symbol `computeHardwareCRC32`
everytime, it will either be there or not when we start, so check it once
during initialization and set the checksum type accordingly.
Then, the loading of `HashAlgorithm` for SSE versions (and ARM equivalent) was
not optimized out, while not necessary. So I reshuffled that part of the code,
which duplicates a tiny bit of code, but ends up in a much cleaner assembly
(and faster as we avoid an extraneous load and some calls).
The following code is the checksum at the end of `scudoMalloc` for x86_64 with
full SSE 4.2, before:
```
mov rax, 0FFFFFFFFFFFFFFh
shl r10, 38h
mov edi, dword ptr cs:_ZN7__scudoL6CookieE ; __scudo::Cookie
and r14, rax
lea rsi, [r13-10h]
movzx eax, cs:_ZN7__scudoL13HashAlgorithmE ; __scudo::HashAlgorithm
or r14, r10
mov rbx, r14
xor bx, bx
call _ZN7__scudo20computeHardwareCRC32Ejm ; __scudo::computeHardwareCRC32(uint,ulong)
mov rsi, rbx
mov edi, eax
call _ZN7__scudo20computeHardwareCRC32Ejm ; __scudo::computeHardwareCRC32(uint,ulong)
mov r14w, ax
mov rax, r13
mov [r13-10h], r14
```
After:
```
mov rax, cs:_ZN7__scudoL6CookieE ; __scudo::Cookie
lea rcx, [rbx-10h]
mov rdx, 0FFFFFFFFFFFFFFh
and r14, rdx
shl r9, 38h
or r14, r9
crc32 eax, rcx
mov rdx, r14
xor dx, dx
mov eax, eax
crc32 eax, rdx
mov r14w, ax
mov rax, rbx
mov [rbx-10h], r14
```
Reviewers: dvyukov, alekseyshl, kcc
Reviewed By: alekseyshl
Subscribers: aemerson, rengolin, llvm-commits
Differential Revision: https://reviews.llvm.org/D32971
llvm-svn: 302538
Summary:
This change adds Android support to the allocator (but doesn't yet enable it in
the cmake config), and should be the last fragment of the rewritten change
D31947.
Android has more memory constraints than other platforms, so the idea of a
unique context per thread would not have worked. The alternative chosen is to
allocate a set of contexts based on the number of cores on the machine, and
share those contexts within the threads. Contexts can be dynamically reassigned
to threads to prevent contention, based on a scheme suggested by @dvyuokv in
the initial review.
Additionally, given that Android doesn't support ELF TLS (only emutls for now),
we use the TSan TLS slot to make things faster: Scudo is mutually exclusive
with other sanitizers so this shouldn't cause any problem.
An additional change made here, is replacing `thread_local` by `THREADLOCAL`
and using the initial-exec thread model in the non-Android version to prevent
extraneous weak definition and checks on the relevant variables.
Reviewers: kcc, dvyukov, alekseyshl
Reviewed By: alekseyshl
Subscribers: srhines, mgorny, llvm-commits
Differential Revision: https://reviews.llvm.org/D32649
llvm-svn: 302300
Summary:
This change introduces scudo_tls.h & scudo_tls_linux.cpp, where we move the
thread local variables used by the allocator, namely the cache, quarantine
cache & prng. `ScudoThreadContext` will hold those. This patch doesn't
introduce any new platform support yet, this will be the object of a later
patch. This also changes the PRNG so that the structure can be POD.
Reviewers: kcc, dvyukov, alekseyshl
Reviewed By: dvyukov, alekseyshl
Subscribers: llvm-commits, mgorny
Differential Revision: https://reviews.llvm.org/D32440
llvm-svn: 301584
Summary:
In the current state of things, the deallocation path puts a chunk in the
Quarantine whether it's enabled or not (size of 0). When the Quarantine is
disabled, this results in the header being loaded (and checked) twice, and
stored (and checksummed) once, in `deallocate` and `Recycle`.
This change introduces a `quarantineOrDeallocateChunk` function that has a
fast path to deallocation if the Quarantine is disabled. Even though this is
not the preferred configuration security-wise, this change saves a sizeable
amount of processing for that particular situation (which could be adopted by
low memory devices). Additionally this simplifies a bit `deallocate` and
`reallocate`.
Reviewers: dvyukov, kcc, alekseyshl
Reviewed By: dvyukov
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D32310
llvm-svn: 301015
Summary:
GetActuallyAllocatedSize is actually expensive. In order to avoid calling this
function in the malloc/free fast path, we change the Scudo chunk header to
store the size of the chunk, if from the Primary, or the amount of unused
bytes if from the Secondary. This way, we only have to call the culprit
function for Secondary backed allocations (and still in realloc).
The performance gain on a singly threaded pure malloc/free benchmark exercising
the Primary allocator is above 5%.
Reviewers: alekseyshl, kcc, dvyukov
Reviewed By: dvyukov
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D32299
llvm-svn: 300861
Summary:
This is part of D31947 that is being split into several smaller changes.
This one deals with all the minor changes, more specifically:
- Rename some variables and functions to make their purpose clearer;
- Reorder some code;
- Mark the hot termination incurring checks as `UNLIKELY`; if they happen, the
program will die anyway;
- Add a `getScudoChunk` method;
- Add an `eraseHeader` method to ScudoChunk that will clear a header with 0s;
- Add a parameter to `allocate` to know if the allocated chunk should be filled
with zeros. This allows `calloc` to not have to call
`GetActuallyAllocatedSize`; more changes to get rid of this function on the
hot paths will follow;
- reallocate was missing a check to verify that the pointer is properly
aligned on `MinAlignment`;
- The `Stats` in the secondary have to be protected by a mutex as the `Add`
and `Sub` methods are actually not atomic;
- The software CRC32 function was moved to the header to allow for inlining.
Reviewers: dvyukov, alekseyshl, kcc
Reviewed By: dvyukov
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D32242
llvm-svn: 300846
Summary:
The local and global quarantine sizes were not offering a distinction for
32-bit and 64-bit platforms. This is addressed with lower values for 32-bit.
When writing additional tests for the quarantine, it was discovered that when
calling some of the allocator interface function prior to any allocation
operation having occured, the test would crash due to the allocator not being
initialized. This was addressed by making sure the allocator is initialized
for those scenarios.
Relevant tests were added in interface.cpp and quarantine.cpp.
Last change being the removal of the extraneous link dependencies for the
tests thanks to rL293220, anf the addition of the gc-sections linker flag.
Reviewers: kcc, alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D29341
llvm-svn: 294037
Summary:
In an effort to getting rid of dependencies to external libraries, we are
replacing atomic PackedHeader use of std::atomic with Sanitizer's
atomic_uint64_t, which allows us to avoid -latomic.
Reviewers: kcc, phosek, alekseyshl
Reviewed By: alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D28864
llvm-svn: 292630
Summary:
ARM & AArch64 runtime detection for hardware support of CRC32 has been added
via check of the AT_HWVAL auxiliary vector.
Following Michal's suggestions in D28417, the CRC32 code has been further
changed and looks better now. When compiled with full relro (which is strongly
suggested to benefit from additional hardening), the weak symbol for
computeHardwareCRC32 is read-only and the assembly generated is fairly clean
and straight forward. As suggested, an additional optimization is to skip
the runtime check if SSE 4.2 has been enabled globally, as opposed to only
for scudo_crc32.cpp.
scudo_crc32.h has no purpose anymore and was removed.
Reviewers: alekseyshl, kcc, rengolin, mgorny, phosek
Reviewed By: rengolin, mgorny
Subscribers: aemerson, rengolin, llvm-commits
Differential Revision: https://reviews.llvm.org/D28574
llvm-svn: 292409
Making this variable non-static avoids the need for locking to ensure
that the initialization is thread-safe which in turns eliminates the
runtime dependency on libc++abi library (for __cxa_guard_acquire and
__cxa_guard_release) which makes it possible to link scudo against
pure C programs.
Differential Revision: https://reviews.llvm.org/D28757
llvm-svn: 292253
Summary:
As raised in D28304, enabling SSE 4.2 for the whole Scudo tree leads to the
emission of SSE 4.2 instructions everywhere, while the runtime checks only
applied to the CRC32 computing function.
This patch separates the CRC32 function taking advantage of the hardware into
its own file, and only enabled -msse4.2 for that file, if detected to be
supported by the compiler.
Another consequence of removing SSE4.2 globally is realizing that memcpy were
not being optimized, which turned out to be due to the -fno-builtin in
SANITIZER_COMMON_CFLAGS. So we now explicitely enable builtins for Scudo.
The resulting assembly looks good, with some CALLs are introduced instead of
the CRC32 code being inlined.
Reviewers: kcc, mgorny, alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D28417
llvm-svn: 291570
Disable the code appending -msse4.2 flag implicitly when the compiler
supports it. The compiler support for this flags do not indicate that
the underlying CPU will support SSE4.2, and passing it may result in
SSE4.2 code being emitted *implicitly*.
If the target platform supports SSE4.2 appropriately, the relevant bits
should be already enabled via -march= or equivalent. In this case
passing -msse4.2 is redundant.
If a runtime detection is desired (which seems to be a case with SCUDO),
then (as gcc manpage points out) the specific SSE4.2 needs to be
isolated into a separate file, the -msse4.2 flag can be forced only
for that file and the function defined in that file can only be called
when the CPU is determined to support SSE4.2.
This fixes SIGILL on SCUDO when it is compiled using gcc-5.4.
Differential Revision: https://reviews.llvm.org/D28304
llvm-svn: 291217
Summary:
With the recent changes to the Secondary, we use less bits for UnusedBytes,
which allows us in return to increase the bits used for Offset. That means
that we can use a Primary SizeClassMap allowing for a larger maximum size.
Reviewers: kcc, alekseyshl
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D27816
llvm-svn: 289838
Summary:
I atually had an integer overflow on 32-bit with D27428 that didn't reproduce
locally, as the test servers would manage allocate addresses in the 0xffffxxxx
range, which led to some issues when rounding addresses.
At this point, I feel that Scudo could benefit from having its own combined
allocator, as we don't get any benefit from the current one, but have to work
around some hurdles (alignment checks, rounding up that is no longer needed,
extraneous code).
Reviewers: kcc, alekseyshl
Subscribers: llvm-commits, kubabrecka
Differential Revision: https://reviews.llvm.org/D27681
llvm-svn: 289572
Summary:
The combined allocator rounds up the requested size with regard to the
alignment, which makes sense when being serviced by the primary as it comes
with alignment guarantees, but not with the secondary. For the rare case of
large alignments, it wastes memory, and entices unnecessarily large fields for
the Scudo header. With this patch, we pass the non-alignement-rounded-up size
to the secondary, and adapt the Scudo code for this change.
Reviewers: alekseyshl, kcc
Subscribers: llvm-commits, kubabrecka
Differential Revision: https://reviews.llvm.org/D27428
llvm-svn: 289088
Summary:
This update introduces i386 support for the Scudo Hardened Allocator, and
offers software alternatives for functions that used to require hardware
specific instruction sets. This should make porting to new architectures
easier.
Among the changes:
- The chunk header has been changed to accomodate the size limitations
encountered on 32-bit architectures. We now fit everything in 64-bit. This
was achieved by storing the amount of unused bytes in an allocation rather
than the size itself, as one can be deduced from the other with the help
of the GetActuallyAllocatedSize function. As it turns out, this header can
be used for both 64 and 32 bit, and as such we dropped the requirement for
the 128-bit compare and exchange instruction support (cmpxchg16b).
- Add 32-bit support for the checksum and the PRNG functions: if the SSE 4.2
instruction set is supported, use the 32-bit CRC32 instruction, and in the
XorShift128, use a 32-bit based state instead of 64-bit.
- Add software support for CRC32: if SSE 4.2 is not supported, fallback on a
software implementation.
- Modify tests that were not 32-bit compliant, and expand them to cover more
allocation and alignment sizes. The random shuffle test has been deactivated
for linux-i386 & linux-i686 as the 32-bit sanitizer allocator doesn't
currently randomize chunks.
Reviewers: alekseyshl, kcc
Subscribers: filcab, llvm-commits, tberghammer, danalbert, srhines, mgorny, modocache
Differential Revision: https://reviews.llvm.org/D26358
llvm-svn: 288255
Summary:
In order to avoid starting a separate thread to return unused memory to
the system (the thread interferes with process startup on Android,
Zygota waits for all threads to exit before fork, but this thread never
exits), try to return it right after free.
Reviewers: eugenis
Subscribers: cryptoad, filcab, danalbert, kubabrecka, llvm-commits
Patch by Aleksey Shlyapnikov.
Differential Revision: https://reviews.llvm.org/D27003
llvm-svn: 288091
Summary:
In order to support 32-bit platforms, we have to make some adjustments in
multiple locations, one of them being the Scudo chunk header. For it to fit on
64 bits (as a reminder, on x64 it's 128 bits), I had to crunch the space taken
by some of the fields. In order to keep the offset field small, the secondary
allocator was changed to accomodate aligned allocations for larger alignments,
hence making the offset constant for chunks serviced by it.
The resulting header candidate has been added, and further modifications to
allow 32-bit support will follow.
Another notable change is the addition of MaybeStartBackgroudThread() to allow
release of the memory to the OS.
Reviewers: kcc
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D25688
llvm-svn: 285209
Summary:
s/CHECK_LT/CHECK_LE/ in the secondary allocator, as under certain circumstances
Ptr + Size can be equal to MapEnd. This edge case was not found by the current
tests, so those were extended to be able to catch that.
Reviewers: kcc
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D25101
llvm-svn: 282913
Summary:
GetActuallyAllocatedSize() was not accounting for the last page of the mapping
being a guard page, and was returning the wrong number of actually allocated
bytes, which in turn would mess up with the realloc logic. Current tests didn't
find this as the size exercised was only serviced by the Primary.
Correct the issue by subtracting PageSize, and update the realloc test to
exercise paths in both the Primary and the Secondary.
Reviewers: kcc
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D24787
llvm-svn: 282030
Summary:
The Sanitizer Secondary Allocator was not entirely ideal was Scudo for several
reasons: decent amount of unneeded code, redundant checks already performed by
the front end, unneeded data structures, difficulty to properly protect the
secondary chunks header.
Given that the second allocator is pretty straight forward, Scudo will use its
own, trimming all the unneeded code off of the Sanitizer one. A significant
difference in terms of security is that now each secondary chunk is preceded
and followed by a guard page, thus mitigating overflows into and from the
chunk.
A test was added as well to illustrate the overflow & underflow situations
into the guard pages.
Reviewers: kcc
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D24737
llvm-svn: 281938
This patch builds on LLVM r279776.
In this patch I've done some cleanup and abstracted three common steps runtime components have in their CMakeLists files, and added a fourth.
The three steps I abstract are:
(1) Add a top-level target (i.e asan, msan, ...)
(2) Set the target properties for sorting files in IDE generators
(3) Make the compiler-rt target depend on the top-level target
The new step is to check if a command named "runtime_register_component" is defined, and to call it with the component name.
The runtime_register_component command is defined in llvm/runtimes/CMakeLists.txt, and presently just adds the component to a list of sub-components, which later gets used to generate target mappings.
With this patch a new workflow for runtimes builds is supported. The new workflow when building runtimes from the LLVM runtimes directory is:
> cmake [...]
> ninja runtimes-configure
> ninja asan
The "runtimes-configure" target builds all the dependencies for configuring the runtimes projects, and runs CMake on the runtimes projects. Running the runtimes CMake generates a list of targets to bind into the top-level CMake so subsequent build invocations will have access to some of Compiler-RT's targets through the top-level build.
Note: This patch does exclude some top-level targets from compiler-rt libraries because they either don't install files (sanitizer_common), or don't have a cooresponding `check` target (stats).
llvm-svn: 279863
Summary:
Currently, the Scudo Hardened Allocator only gets its flags via the SCUDO_OPTIONS environment variable.
With this patch, we offer the opportunity for programs to define their own options via __scudo_default_options() which behaves like __asan_default_options() (weak symbol).
A relevant test has been added as well, and the documentation updated accordingly.
I also used this patch as an opportunity to rename a few variables to comply with the LLVM naming scheme, and replaced a use of Report with dieWithMessage for consistency (and to avoid a callback).
Reviewers: llvm-commits, kcc
Differential Revision: https://reviews.llvm.org/D23018
llvm-svn: 277536
Summary:
This patch is a refactoring of the way cmake 'targets' are grouped.
It won't affect non-UI cmake-generators.
Clang/LLVM are using a structured way to group targets which ease
navigation through Visual Studio UI. The Compiler-RT projects
differ from the way Clang/LLVM are grouping targets.
This patch doesn't contain behavior changes.
Reviewers: kubabrecka, rnk
Subscribers: wang0109, llvm-commits, kubabrecka, chrisha
Differential Revision: http://reviews.llvm.org/D21952
llvm-svn: 275111
Summary:
This is an initial implementation of a Hardened Allocator based on Sanitizer Common's CombinedAllocator.
It aims at mitigating heap based vulnerabilities by adding several features to the base allocator, while staying relatively fast.
The following were implemented:
- additional consistency checks on the allocation function parameters and on the heap chunks;
- use of checksum protected chunk header, to detect corruption;
- randomness to the allocator base;
- delayed freelist (quarantine), to mitigate use after free and overall determinism.
Additional mitigations are in the works.
Reviewers: eugenis, aizatsky, pcc, krasin, vitalybuka, glider, dvyukov, kcc
Subscribers: kubabrecka, filcab, llvm-commits
Differential Revision: http://reviews.llvm.org/D20084
llvm-svn: 271968
Peter Collingbourne