Bug 169290 – WebAssembly: Make OOB for fast memory do an extra safety check by ensuring the faulting address is in the range we allocated for fast memory

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Keith Miller]

I can take this since wrote the code anyway.

[Keith Miller]

Created attachment 303824 [details]
Patch

[Saam Barati]

Comment on attachment 303824 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303824&action=review

Comment below, otherwise, LGTM.
r- just because of my comment.

> Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:104
> +        {
> +            void* faultingAddress = sigInfo->si_addr;
> +            dataLogLnIf(verbose, "checking faulting address: ", RawPointer(faultingAddress), " is in an active fast memory");
> +            LockHolder locker(memoryLock);
> +            auto& activeFastMemories = viewActiveFastMemories(locker);
> +            for (void* activeMemory : activeFastMemories) {
> +                dataLogLnIf(verbose, "checking fast memory at: ", RawPointer(activeMemory));
> +                if (activeMemory <= faultingAddress && faultingAddress < static_cast<char*>(activeMemory) + fastMemoryMappedBytes) {
> +                    faultedInActiveFastMemory = true;

You should first check that we're in JIT code otherwise we could've crashed while holding the lock.

[Saam Barati]

Comment on attachment 303824 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=303824&action=review

>> Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:104
>> +                    faultedInActiveFastMemory = true;
> 
> You should first check that we're in JIT code otherwise we could've crashed while holding the lock.

Ignore me, I'm wrong.

[Saam Barati]

Comment on attachment 303824 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 303824 [details]
Patch

Clearing flags on attachment: 303824

Committed r213599: <http://trac.webkit.org/changeset/213599>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.