Bug 169290

Summary: 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
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Keith Miller <keith_miller>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, fpizlo, ggaren, gskachkov, jfbastien, keith_miller, mark.lam, msaboff, ticaiolima, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Saam Barati
Reported 2017-03-07 10:44:35 PST
...
Attachments
Patch (9.86 KB, patch)
2017-03-08 11:47 PST, Keith Miller 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Keith Miller
Comment 1 2017-03-08 09:32:51 PST
I can take this since wrote the code anyway.
Keith Miller
Comment 2 2017-03-08 11:47:35 PST
Created attachment 303824 [details] Patch
Saam Barati
Comment 3 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.
Saam Barati
Comment 4 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.
Saam Barati
Comment 5 2017-03-08 14:23:11 PST
Comment on attachment 303824 [details] Patch r=me
WebKit Commit Bot
Comment 6 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>
WebKit Commit Bot
Comment 7 2017-03-08 14:51:00 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.