[12] RegExp::byteCodeCompileIfNecessary not thread-safe vs concurrent compiler thread

Severity: Medium | Component: JSC Yarr regex engine | dcf25ed

Rated Medium because the diff fixes a TOCTOU race between the mutator thread and a JSC concurrent compiler thread on a std::unique_ptr<BytecodePattern> field of RegExp; under interleaving the race produces undefined behavior, but the trigger requires very narrow timing the commit message confirms is not reachable without artificial sleeps.

RegExp::matchConcurrently shouldn't cause any compilation, but bail out if JIT code for the regexp doesn't already exist. However, it is possible that a RegExp has JIT code but no bytecode, in which case matchConcurrently can incorrectly racily attempt to compile bytecode.

Source/JavaScriptCore/runtime/RegExp.cpp

Source/JavaScriptCore/runtime/RegExpInlines.h

Patch Details

Two changes: byteCodeCompileIfNecessary now takes the cell lock (Locker locker { cellLock() }) before checking and writing m_regExpBytecode. In both overloads of matchInline, the JIT-failure fallback path is restructured: the call to byteCodeCompileIfNecessary is now gated by if constexpr (matchFrom == Yarr::MatchFrom::VMThread), and an additional if (!m_regExpBytecode) return -1; bails the compiler thread out when bytecode has not been produced. The commit message states the in-matchInline check intentionally does not take the cell lock because the caller matchConcurrently already holds it.

TOCTOU race on a lazily-compiled cell member between the mutator thread and a JSC concurrent compiler thread that was supposed to be read-only.

Background

JSC compiles a regular expression lazily into two possible forms: Yarr JIT machine code (m_regExpJITCode) for patterns the JIT can handle, and a Yarr bytecode pattern (m_regExpBytecode) interpreted by Yarr::interpret. At runtime, JIT-compiled regex code can return Yarr::JSRegExpResult::JITCodeFailure for inputs it cannot handle, at which point execution punts to the bytecode interpreter. RegExp::matchConcurrently is used by JSC's concurrent compiler threads (DFG/FTL) — when the optimising compiler tries to constant-fold or speculate on a regex match during compilation, it may invoke matching from a thread other than the main VM thread. The convention is that concurrent compiler threads observe heap state without mutating it. cellLock() is a per-cell lock used to synchronise rare cases where mutator-side mutation must be visible to compiler-thread reads. MatchFrom is a template parameter distinguishing VMThread from CompilerThread calling contexts.

Analysis

RegExp::matchConcurrently runs on a JSC concurrent compiler thread and is intended to read pre-existing JIT code only, not to mutate the RegExp cell. However, a RegExp can be in a state where JIT code exists but the bytecode does not. If the JIT returns JSRegExpResult::JITCodeFailure, the previous code unconditionally called byteCodeCompileIfNecessary on whichever thread was running — including the compiler thread. Meanwhile, the mutator thread executing a regular regex match on the same RegExp could be entering the same path. Two threads could thus check if (m_regExpBytecode) simultaneously, both see null, and both proceed to allocate and assign a new BytecodePattern to the std::unique_ptr member.

The race window covers an unsynchronised TOCTOU on the existence check followed by a non-atomic write to a unique_ptr field — classic concurrent modification of an owning smart pointer. The consequence is either double-allocation (leak), a torn write that one thread observes mid-update, or — most concerning — one thread overwriting a unique_ptr whose previous value the other thread is still operating on, leading to use-after-free of the freed BytecodePattern while it is being interpreted.

This vulnerability weakened the thread-safety invariant that RegExp cell state is mutated only by the mutator thread while concurrent JIT compilers observe it read-only. Concurrent modification of a unique_ptr field inside a JSC cell under standard C++ memory semantics is undefined behaviour and would yield use-after-free or torn-write conditions on m_regExpBytecode. The fix uses cellLock() for serialisation plus a compile-time matchFrom gate to prevent the compiler thread from initiating compilation at all — the gating pattern is the more robust half, because it removes the mutation from the compiler-thread side rather than serialising it. Anywhere JSC has a lazy-compilation member on a cell that may be touched from a concurrent compiler thread, the "check then write" pattern is a latent race.

Audit directions

• Lazy compilation/initialisation of cell members reachable from JSC concurrent compiler threads. Audit every member of every cell that is populated on first use (bytecode caches, sample-string caches, derived-structure caches) and is reachable through any matchConcurrently-style path or through DFG/FTL AbstractInterpreter constant folding. Grep for matchConcurrently, Yarr::MatchFrom::CompilerThread, and any XxxIfNecessary/compileIfNecessary/ensureXxx helpers on JSCell subclasses; for each, check whether the helper is callable from a non-mutator context and whether it takes cellLock().
• MatchFrom-style thread-context template parameters that gate behaviour at compile time. Audit other JSC primitives that have such parameters (Yarr, JSGlobalObject lookups, structure transitions) and verify every mutating operation inside the templated function is statically excluded from the non-mutator instantiation, not merely "unlikely" at runtime.
• std::unique_ptr-typed members on JSCell subclasses written without synchronisation. Grep Source/JavaScriptCore/runtime and Source/JavaScriptCore/yarr for std::unique_ptr<...> m_ declarations on cell types and verify each write site either holds cellLock() or is provably mutator-only.
• Investigate the parallel-compiler entry points: confirm that matchConcurrently is the only Yarr path callable from compiler threads, and that the same JIT-failure punt does not exist elsewhere (e.g., in RegExpObject operations) without an equivalent MatchFrom-style gate.