Package: libwebkitgtk-3.0-0
Version: 1.8.1-3.3
Severity: grave
Tags: patch


Machine: Dell PowerEdge 3250
Processor: 2x Itanium Madison 1.5GHz 6M
Memory: 16G



I realized this bug while working on bug#642750.

The Epiphany browser crashed with a SIGSEGV in JSC::JSArray::increaseVectorLength()

I built the libwebkitgtk-3.0-0 package which was configured with --enable-debug. Furthermore, I modified the Source/JavaScriptCore/wtf/Assertions.h in order to be able to continue subsequent to a failed assertion; I defined CRASH() to expand to nothing.



bool JSArray::increaseVectorLength(JSGlobalData& globalData, unsigned newLength)
{
// This function leaves the array in an internally inconsistent state, because it does not move any values from sparse value map // to the vector. Callers have to account for that, because they can do it more efficiently.
    if (newLength > MAX_STORAGE_VECTOR_LENGTH)
        return false;

    ArrayStorage* storage = m_storage;

    unsigned vectorLength = m_vectorLength;
    ASSERT(newLength > vectorLength);
    unsigned newVectorLength = getNewVectorLength(newLength);

// Fast case - there is no precapacity. In these cases a realloc makes sense.
    if (LIKELY(!m_indexBias)) {
        void* newStorage = storage->m_allocBase;
if (!globalData.heap.tryReallocateStorage(&newStorage, storageSize(vectorLength), storageSize(newVectorLength)))
            return false;

storage = m_storage = reinterpret_cast_ptr<ArrayStorage*>(static_cast<char*>(newStorage));
        m_storage->m_allocBase = newStorage;
        ASSERT(m_storage->m_allocBase);

        WriteBarrier<Unknown>* vector = storage->m_vector;
        for (unsigned i = vectorLength; i < newVectorLength; ++i)
vector[i].clear(); <=================== here the crash occurs

        m_vectorLength = newVectorLength;

        return true;
    }


It turned out that tryReallocateStorage() allocated a memory block that is smaller than requested with the last parameter of the function. When it occured, the requested size was quite large and the code of the following function was executed (Source/JavaScriptCore/heap/CopiedSpaceInlineMethods.h):

inline CheckedBoolean CopiedSpace::tryAllocateOversize(size_t bytes, void** outPtr)
{
    ASSERT(isOversize(bytes));

size_t blockSize = WTF::roundUpToMultipleOf<s_pageSize>(sizeof(CopiedBlock) + bytes); PageAllocationAligned allocation = PageAllocationAligned::allocate(blockSize, s_pageSize, OSAllocator::JSGCHeapPages);
    if (!static_cast<bool>(allocation)) {
        *outPtr = 0;
        return false;
    }
....
}

WTF::roundUpToMultipleOf<s_pageSize>() rounded up the requested size to a multiple of 4K. s_pageSize is a constant which is defined in the CopiedSpace class (Source/JavaScriptCore/heap/CopiedSpace.h)

    static const size_t s_pageSize = 4 * KB;


At next PageAllocationAligned::allocate() is called (Source/JavaScriptCore/wtf/PageAllocationAligned.cpp):

PageAllocationAligned PageAllocationAligned::allocate(size_t size, size_t alignment, OSAllocator::Usage usage, bool writable, bool executable)
{
    ASSERT(isPageAligned(size));
    ASSERT(isPageAligned(alignment));
    ASSERT(isPowerOfTwo(alignment));
    ASSERT(size >= alignment);
    size_t alignmentMask = alignment - 1;

#if OS(DARWIN)
    ....
#else
    size_t alignmentDelta = alignment - pageSize();

    // Resererve with suffcient additional VM to correctly align.
    size_t reservationSize = size + alignmentDelta;
void* reservationBase = OSAllocator::reserveUncommitted(reservationSize, usage, writable, executable);

    // Select an aligned region within the reservation and commit.
void* alignedBase = reinterpret_cast<uintptr_t>(reservationBase) & alignmentMask ? reinterpret_cast<void*>((reinterpret_cast<uintptr_t>(reservationBase) & ~alignmentMask) + alignment)
        : reservationBase;
    OSAllocator::commit(alignedBase, size, writable, executable);

return PageAllocationAligned(alignedBase, size, reservationBase, reservationSize);
#endif
}


The first two assertions failed:
    ASSERT(isPageAligned(size));
    ASSERT(isPageAligned(alignment));

The alignmentDelta variable is a 64-bits unsigned integer and evaluated to a value of 18446744073709539328 (which is 2^64 - 12288 ) because at the line

    size_t alignmentDelta = alignment - pageSize();

the 'alignment' arg is 4096 and 'pageSize()' returned 16384 - we'll take a closer look on the latter one below.
The subsequent line

    size_t reservationSize = size + alignmentDelta;

evaluated the reservationSize var to an integer value less than size because the 64-bits integer arithmetics overflowed and wrapped around.
This is the reason why the allocated memory block was too small.

The mentioned pageSize() function returned the actual page size of 16K, which is correct. Linux on ia64 can have 4K, 8K, 16K, or 64K page dependant on the configuration the Kernel was compiled with. Debian uses 16K on ia64.

The mentioned pageSize() function is in Source/JavaScriptCore/wtf/PageBlock.cpp:

static size_t s_pageSize;

#if OS(UNIX)

inline size_t systemPageSize()
{
    return getpagesize();
}

#elif OS(WINDOWS)

inline size_t systemPageSize()
{
    static size_t size = 0;
    SYSTEM_INFO system_info;
    GetSystemInfo(&system_info);
    size = system_info.dwPageSize;
    return size;
}

#endif

size_t pageSize()
{
    if (!s_pageSize)
        s_pageSize = systemPageSize();
    ASSERT(isPowerOfTwo(s_pageSize));
    return s_pageSize;
}



So pageSize() returns the actual memory page size that the operating system reports.

(Don't confuse the (constant) static s_pageSize member of the the CopiedSpace class and the static s_pageSize var of PageBlock.cpp.)

The problem is that webkit uses a mixture of hard-coded page size values and of the actual page size value that the OS reports. If they are different, webkit will crash.

Since you don't know the page size upon the compile time (of webkit) on some archs, for example, ia64, the proposed patch removes any hard-coded page size assumptions on the considered code. The patched CopiedSpace class uses the page size which the OS reports.


I built the libwebkitgtk-3.0-0 with the patch of this bug report and the one of bug#642750); the Epiphany browser does no longer crash within JSC::JSArray::increaseVectorLength().
The patch is for the most recent libwebkitgtk-3.0-0 package of Wheezy.

The patches also fix bug#582774 (seed FTBFS on ia64).

The bug affects any arch that uses a page size larger than 4K. If the next page adjacent to the (re)allocated memory was already mapped (for another data), JSArray::increaseVectorLength() might not crash but overwrite some other data; this bug can cause SIGSEGVs on other code locations with completely different stack traces. I think it is worth to check whether this patch solves some other bug reports, for example, on MIPS.

Stephan

Attachment: large-mem-page.patch
Description: large-mem-page.patch

Reply via email to