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
Summary: WebAssembly: Make OOB for fast memory do an extra safety check by ensuring th...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-07 10:44 PST by Saam Barati
Modified: 2017-03-08 14:51 PST (History)
11 users (show)

See Also:


Attachments
Patch (9.86 KB, patch)
2017-03-08 11:47 PST, Keith Miller
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2017-03-07 10:44:35 PST
...
Comment 1 Keith Miller 2017-03-08 09:32:51 PST
I can take this since wrote the code anyway.
Comment 2 Keith Miller 2017-03-08 11:47:35 PST
Created attachment 303824 [details]
Patch
Comment 3 Saam Barati 2017-03-08 14:19:56 PST
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.
Comment 4 Saam Barati 2017-03-08 14:21:34 PST
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.
Comment 5 Saam Barati 2017-03-08 14:23:11 PST
Comment on attachment 303824 [details]
Patch

r=me
Comment 6 WebKit Commit Bot 2017-03-08 14:50:54 PST
Comment on attachment 303824 [details]
Patch

Clearing flags on attachment: 303824

Committed r213599: <http://trac.webkit.org/changeset/213599>
Comment 7 WebKit Commit Bot 2017-03-08 14:51:00 PST
All reviewed patches have been landed.  Closing bug.