Bug 227869 - Invalid machine code emitted by SpeculativeJIT::emitObjectOrOtherBranch
Summary: Invalid machine code emitted by SpeculativeJIT::emitObjectOrOtherBranch
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Minor
Assignee: Robin Morisset
Keywords: InRadar
Depends on:
Reported: 2021-07-12 04:22 PDT by Samuel Groß
Modified: 2021-07-13 20:21 PDT (History)
10 users (show)

See Also:

Patch (2.61 KB, patch)
2021-07-12 15:52 PDT, Robin Morisset
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Groß 2021-07-12 04:22:50 PDT
Fuzzilli found the following sample which sometimes crashes a recent build of JSC (commit https://github.com/WebKit/WebKit/commit/36c74cfbcf3e8ced764bbb06d01a725610cc1948):

    function main() {
    let v2 = 0;
    do {
        for (let v19 = 0; v19 < 10000; v19++) {
            function* v21(v22,v23,v24) {
            const v26 = class V26 extends Object {
                constructor(v28,v29) {
                    do {
                        const v31 = 8 * this;
                    } while (v2 < 8);
                    const v32 = Object();
            function v36(v37,v38) {
                if (v21) {
                    const v41 = Object();
                } else {
                    const v71 = Object;
            const v73 = new Promise(v36);
        const v74 = v2++;
        print("Invalidated masqueradesAsUndefinedWatchpoint");
    } while (v2 < Object);
    // Wait for JIT thread to finish?
    // STDERR:
    // ASSERTION FAILED: r >= 0
    // /home/builder/webkit/Source/JavaScriptCore/assembler/X86Assembler.h(4113) : void JSC::X86Assembler::X86InstructionFormatter::SingleInstructionBufferWriter::emitRex(bool, int, int, int)

The bug is due to a race between a JIT compiler thread checking the state of a watchpoint and the main thread invalidating that watchpoint. The following patch can be used to trigger the race reliably:

    diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
    index 2ea95177b9bf..3b943b0e146e 100644
    --- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
    +++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
    @@ -1992,6 +1992,9 @@ void SpeculativeJIT::emitObjectOrOtherBranch(Edge nodeUse, BasicBlock* taken, Ba
             structureGPR = structure.gpr();
    +    puts("Waiting in SpeculativeJIT::emitObjectOrOtherBranch. Press <enter> after the masqueradesAsUndefinedWatchpoint was invalidated.");
    +    getchar();
         MacroAssembler::Jump notCell = m_jit.branchIfNotCell(JSValueRegs(valueGPR));
         if (masqueradesAsUndefinedWatchpointIsStillValid()) {

The issue is that SpeculativeJIT::emitObjectOrOtherBranch checks the state of the masqueradesAsUndefinedWatchpoint twice, the first time initializing a local register only if the watchpoint is invalid, the second time emitting code using that register if the watchpoint is invalid. If the state of the watchpoint changes in between the two queries, then the compiler will emit code with an invalid register number (-1), leading to corrupted and somewhat controllable machine code being emitted (or, in debug builds, an assertion failure). However, I do not believe that this bug is exploitable because the WatchpointCollectionPhase will watch the masqueradesAsUndefinedWatchpoint prior to the SpeculativeJIT emitting the invalid code. As such, as far as I can tell, the corrupted machine code is never installed, and can thus never be executed at runtime, as the watchpoint will have been invalidated by then. I'm still marking this issue as restricted out of precaution, but feel free to derestrict it if you share my assessment.
Comment 1 Radar WebKit Bug Importer 2021-07-12 04:23:01 PDT
Comment 2 Robin Morisset 2021-07-12 14:58:15 PDT
Thank you for this bug report, and the great analysis.

I agree with you, both on the root cause and on the fact that is should not be exploitable.
Comment 3 Robin Morisset 2021-07-12 15:52:53 PDT
Created attachment 433365 [details]
Comment 4 Mark Lam 2021-07-12 15:57:57 PDT
Comment on attachment 433365 [details]

Comment 5 EWS 2021-07-13 20:21:10 PDT
Committed r279903 (239652@main): <https://commits.webkit.org/239652@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433365 [details].