Bug 33905

Summary: Corruption of memory allocated by TCMalloc_SystemAlloc on OS with 8k pages
Product: WebKit Reporter: Stephen Clarke <stephen.clarke>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: UNCONFIRMED ---    
Severity: Normal CC: annulen
Priority: P2    
Version: 420+   
Hardware: Other   
OS: Linux   
Attachments:
Description Flags
Patch to fix mmap call none

Description Stephen Clarke 2010-01-20 09:13:58 PST
Created attachment 47040 [details]
Patch to fix mmap call

We are seeing corruption (zeroing) of memory that has been allocated by TCMalloc_SystemAlloc and is still in use.  We observe this using the demo browser in Qt release 4.6.0 when attempting to run the sunspider-0.9 javascript benchmarks.  Our platform is a proprietary 32-bit architecture running linux based on the kernel version 2.6.24.

We have discovered that this memory is being "released" to the OS using the HAVE_MMAP-version of TCMalloc_SystemRelease.  The call to TCMalloc_SystemRelease is from the background memory scavenging thread, specifically the function TCMalloc_PageHeap::scavenge().

One slightly unusual feature of our system is that it uses an 8K page size.  However, the background scavenger thread scavenges 4kByte chunks of memory.  In the case where we observe memory corruption, the scavenger is attempting to scavenge the first 4k from an 8K page, when the remainder of the page is still in use.  The scavenger calls TCMalloc_SystemRelease with a length of 4kBytes.  But TCMalloc_SystemRelease calls mmap(), which releases the whole page, and so the in-use half of the page is also released.  When this page is later reused, the system zero-initializes all 8K of it, thus corrupting the 4k that is still in use.

The description of TCMalloc_SystemRelease() in TCSystemAlloc.h clearly states the intention:  "Only pages fully covered by the memory region will be released, partial pages will not.", so it should not cause all 8K to be released.

But The specification of mmap (from http://www.opengroup.org/onlinepubs/009695399/functions/mmap.html) states:
"The mapping established by mmap() shall replace any previous mappings for those whole pages containing any part of the address space of the process starting at pa and continuing for len bytes."
So if mmap() is given a 4K region, it will expand this to the whole 8K page containing that region.

It seems to me that the HAVE_MMAP-version of TCMalloc_SystemRelease() should remove partial pages from the region it passes to mmap().  Indeed the HAVE(MADV_FREE)-version of TCMalloc_SystemRelease() already does exactly this.  We have applied the attached patch, and it fixes the problem we are observing.
Comment 1 Alexey Proskuryakov 2010-06-12 16:50:10 PDT
This patch has been overlooked, because it isn't marked for review. Please see <http://webkit.org/coding/contributing.html> for more information.
Comment 2 Konstantin Tokarev 2011-07-20 05:32:59 PDT
Could anyone review this patch?