Commit 5db7b125 authored by Findit's avatar Findit

Revert "[PartitionAlloc] Zero on free."

This reverts commit 4363f493.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 816834 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzQzNjNmNDkzNDNkZjgyZDNiOWJiMmZiYmI1NWJjNDQ0OGVhYmZmNGEM

Sample Failed Build: https://ci.chromium.org/b/8866507600045542768

Sample Failed Step: compile

Original change's description:
> [PartitionAlloc] Zero on free.
> 
> This CL 'zaps' (`memset`s) the allocated region on free. This may
> help detect some instances of UAF, causing them to crash (with
> nullptr deref).
> 
> Rather than doing so on every free, we zap probabilistically to
> reduce the performance impact.
> 
> If the perf bots show an unacceptable regression, we'll revert this.
> But, it might be acceptable.
> 
> Bug: 1005070, 1013329, 1013326, 1013324
> Change-Id: Ia36ee17625290646216985dd6fdffabfa84a9ba2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873272
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Reviewed-by: Bartek Nowierski <bartekn@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816834}


Change-Id: I3ebcc5179fa0830faa5c9df8dae63a562cdb0b1b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1005070, 1013329, 1013326, 1013324
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2469840
Cr-Commit-Position: refs/heads/master@{#816836}
parent 923bb99a
...@@ -92,10 +92,6 @@ ...@@ -92,10 +92,6 @@
#include <stdlib.h> #include <stdlib.h>
#endif #endif
#if defined(OS_WIN)
#include <windows.h>
#endif
#if defined(ADDRESS_SANITIZER) #if defined(ADDRESS_SANITIZER)
#include <sanitizer/asan_interface.h> #include <sanitizer/asan_interface.h>
#endif // defined(ADDRESS_SANITIZER) #endif // defined(ADDRESS_SANITIZER)
...@@ -110,26 +106,6 @@ ...@@ -110,26 +106,6 @@
PA_CHECK(false); \ PA_CHECK(false); \
} }
namespace {
// Returns true if we've hit the end of a random-length period. We don't want to
// invoke `RandomValue` too often, because we call this function in a hot spot
// (`Free`), and `RandomValue` incurs the cost of atomics.
#if !DCHECK_IS_ON()
ALWAYS_INLINE bool RandomPeriod() {
static thread_local uint8_t counter = 0;
if (UNLIKELY(counter == 0)) {
// It's OK to truncate this value.
counter = static_cast<uint8_t>(base::RandomValue());
}
// If `counter` is 0, this will wrap. That is intentional and OK.
counter--;
return counter == 0;
}
#endif
} // namespace
namespace base { namespace base {
typedef void (*OomFunction)(size_t); typedef void (*OomFunction)(size_t);
...@@ -503,7 +479,10 @@ ALWAYS_INLINE void PartitionRoot<thread_safe>::FreeNoHooksImmediate( ...@@ -503,7 +479,10 @@ ALWAYS_INLINE void PartitionRoot<thread_safe>::FreeNoHooksImmediate(
PA_DCHECK(page); PA_DCHECK(page);
PA_DCHECK(IsValidPage(page)); PA_DCHECK(IsValidPage(page));
const size_t utilized_slot_size = page->GetUtilizedSlotSize(); #if DCHECK_IS_ON()
size_t utilized_slot_size = page->GetUtilizedSlotSize();
#endif
if (allow_extras) { if (allow_extras) {
// |ptr| points after the tag and the cookie. // |ptr| points after the tag and the cookie.
// //
...@@ -569,17 +548,6 @@ ALWAYS_INLINE void PartitionRoot<thread_safe>::FreeNoHooksImmediate( ...@@ -569,17 +548,6 @@ ALWAYS_INLINE void PartitionRoot<thread_safe>::FreeNoHooksImmediate(
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
memset(ptr, kFreedByte, utilized_slot_size); memset(ptr, kFreedByte, utilized_slot_size);
#else
// `memset` only once in a while: we're trading off safety for time
// efficiency.
if (UNLIKELY(RandomPeriod()) && !page->bucket->is_direct_mapped()) {
#if defined(OS_WIN)
SecureZeroMemory(ptr, allocated_size);
#else
// TODO(palmer): Use an equivalent of memset_s.
memset(ptr, 0, utilized_slot_size);
#endif
}
#endif #endif
// TLS access can be expensive, do a cheap local check first. // TLS access can be expensive, do a cheap local check first.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment