Patch forthcoming.
Created attachment 337126 [details] the patch
Comment on attachment 337126 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=337126&action=review > Source/JavaScriptCore/ChangeLog:-2936 > -2018-03-09 Filip Pizlo <fpizlo@apple.com> > - > - Split DirectArguments into JSValueOOB and JSValueStrict parts > - https://bugs.webkit.org/show_bug.cgi?id=183458 > - > - Reviewed by Yusuke Suzuki. > - > - Our Spectre plan for JSValue objects is to allow inline JSValue stores and loads guarded by > - unmitigated structure checks. This works because objects reachable from JSValues (i.e. JSValue > - objects, like String, Symbol, and any descendant of JSObject) will only contain fields that it's OK > - to read and write within a Spectre mitigation window. Writes are important, because within the > - window, a write could appear to be made speculatively and rolled out later. This means that: > - > - - JSValue objects cannot have lengths, masks, or anything else inline. > - > - - JSValue objects cannot have an inline type that is used as part of a Spectre mitigation for a type > - check, unless that type is in the form of a poison key. > - > - This means that the dynamic poisoning that I previously landed for DirectArguments is wrong. It also > - means that it's wrong for DirectArguments to have an inline length. > - > - This changes DirectArguments to use poisoning according to the universal formula: > - > - - The random accessed portions are out-of-line, pointed to by a poisoned pointer. > - > - - No inline length. > - > - Surprisingly, this is perf-neutral. It's probably perf-neutral because our compiler optimizations > - amortize whatever cost there was. > - I'll revert this.
Created attachment 337128 [details] the patch
Attachment 337128 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:123: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 337132 [details] the patch
Attachment 337132 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:123: Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(). [security/assertion] [5] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 337132 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=337132&action=review rs=me > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:15570 > + LValue dynamicPoison(LValue value, LValue poison) Do we still need these?
Landed in https://trac.webkit.org/changeset/230266/webkit
<rdar://problem/39180055>
(In reply to Saam Barati from comment #7) > Comment on attachment 337132 [details] > the patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=337132&action=review > > rs=me > > > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:15570 > > + LValue dynamicPoison(LValue value, LValue poison) > > Do we still need these? Probably not. We might still use them for scoped arguments poisoning or something. I'll kill them once I'm done killing other things.