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

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

RegExp의 std::unique_ptr<BytecodePattern> 필드를 둘러싸고 mutator thread와 JSC concurrent compiler thread 사이에서 발생하는 TOCTOU race를 수정한 diff이기 때문에 Medium으로 평가되었습니다. 두 thread가 interleave되면 undefined behavior가 발생합니다. 다만 이를 유발하려면 매우 좁은 timing window가 필요하며, commit message에서도 인위적인 sleep 없이는 재현할 수 없다고 확인하고 있습니다.

RegExp::matchConcurrently는 컴파일을 유발해서는 안 됩니다. regexp의 JIT code가 존재하지 않는다면, 컴파일하는 대신 bail out하는 것이 올바른 동작입니다. 그러나 RegExp에 JIT code는 있지만 bytecode가 없는 상태가 가능합니다. 이 경우 matchConcurrently가 race 조건 속에서 bytecode 컴파일을 부적절하게 시도하는 상황이 발생할 수 있습니다.

Source/JavaScriptCore/runtime/RegExp.cpp

 void RegExp::byteCodeCompileIfNecessary(VM* vm)
 {
+    Locker locker { cellLock() };
+
     if (m_regExpBytecode)
         return;

Source/JavaScriptCore/runtime/RegExpInlines.h

         if (result == static_cast<int>(Yarr::JSRegExpResult::JITCodeFailure)) {
-            // JIT code가 expression을 처리할 수 없어 interpreter로 전달합니다.
-            byteCodeCompileIfNecessary(&vm);
-            if (m_state == ParseError)
-                return throwError();
+            // bytecode 컴파일은 mutator만 수행할 수 있습니다. compiler thread는 이미 존재하는
+            // bytecode를 사용해야 하며, bytecode가 없으면 bail out합니다.
+            if constexpr (matchFrom == Yarr::MatchFrom::VMThread) {
+                byteCodeCompileIfNecessary(&vm);
+                if (m_state == ParseError)
+                    return throwError();
+            }
+            if (!m_regExpBytecode)
+                return -1;

Patch Details

변경 사항은 두 가지입니다. 먼저 byteCodeCompileIfNecessary에서는 m_regExpBytecode를 확인하거나 쓰기 전에 cell lock(Locker locker { cellLock() })을 취득하도록 수정되었습니다. 한편 matchInline의 두 overload에서는 JIT 실패 시 fallback 경로가 재구성되었는데, byteCodeCompileIfNecessary 호출은 if constexpr (matchFrom == Yarr::MatchFrom::VMThread) 조건으로 gating되었습니다. bytecode가 생성되지 않은 경우 compiler thread가 bail out하도록 if (!m_regExpBytecode) return -1;도 함께 추가되었습니다. commit message에 따르면 matchInline 내부의 check는 의도적으로 cell lock을 취득하지 않습니다. 호출자인 matchConcurrently가 이미 lock을 보유하고 있기 때문입니다.

읽기 전용으로 동작해야 하는 JSC concurrent compiler thread와 mutator thread 사이에서, lazily compile된 cell 멤버를 둘러싼 TOCTOU race.

Background

JSC는 정규식을 두 가지 형태로 lazy하게 컴파일합니다. JIT가 처리할 수 있는 패턴은 Yarr JIT machine code(m_regExpJITCode)로 컴파일됩니다. 나머지 패턴에 대해서는 Yarr::interpret가 해석하는 Yarr bytecode pattern(m_regExpBytecode)이 생성됩니다. 런타임에서 JIT로 컴파일된 regex code는 처리할 수 없는 입력에 대해 Yarr::JSRegExpResult::JITCodeFailure를 반환할 수 있습니다. 이 경우 실행은 bytecode interpreter로 전달됩니다. RegExp::matchConcurrently는 JSC의 concurrent compiler thread(DFG/FTL)에서 사용됩니다. 최적화 컴파일러가 컴파일 중에 regex match를 constant-fold하거나 speculation을 시도할 때, main VM thread가 아닌 별도의 thread에서 matching이 호출될 수 있습니다. concurrent compiler thread는 heap 상태를 변경하지 않고 관찰만 해야 한다는 규약이 있습니다. cellLock()은 cell 단위의 lock으로, mutator 측의 변경이 compiler thread 읽기에 가시적이어야 하는 드문 상황을 동기화하기 위해 사용됩니다. MatchFrom은 호출 컨텍스트가 VMThread인지 CompilerThread인지를 구분하는 template parameter입니다.

Analysis

RegExp::matchConcurrently는 JSC concurrent compiler thread에서 실행되며, 기존의 JIT code를 읽는 것만을 목적으로 합니다. RegExp cell을 변경하는 것은 의도된 동작이 아닙니다. 그러나 JIT code는 존재하지만 bytecode는 없는 상태의 RegExp가 존재할 수 있습니다. 이 상황에서 JIT가 JSRegExpResult::JITCodeFailure를 반환하면, 수정 전 코드는 실행 중인 thread에 관계없이 byteCodeCompileIfNecessary를 무조건 호출했습니다. compiler thread도 예외가 아니었습니다. 한편 동일한 RegExp에 대해 일반적인 regex match를 실행하는 mutator thread도 같은 경로에 진입할 수 있습니다. 결과적으로 두 thread가 동시에 if (m_regExpBytecode)를 확인하는 상황이 발생할 수 있습니다. 양쪽 모두 null을 관찰하면, 각자 새로운 BytecodePattern을 할당하여 std::unique_ptr 멤버에 대입하게 됩니다.

race window는 존재 여부 확인의 비동기화된 TOCTOU와, 이어지는 unique_ptr 필드에 대한 non-atomic write를 포함합니다. 소유권을 가진 smart pointer를 concurrent하게 수정하는 전형적인 패턴입니다. 결과는 세 가지 형태로 나타날 수 있습니다. 먼저 이중 할당으로 인한 memory leak이 발생할 수 있습니다. 또한 한 thread가 update 도중의 중간 상태를 관찰하는 torn write가 성립할 수 있습니다. 가장 심각한 경우는, 한 thread가 다른 thread가 여전히 사용 중인 unique_ptr의 이전 값을 덮어씌우는 상황입니다. 이 경우 interpreter가 해제된 BytecodePattern을 사용하는 도중 use-after-free가 발생할 수 있습니다.

이 취약점은 RegExp cell 상태는 mutator thread만 변경하고 concurrent JIT compiler는 read-only로 관찰한다는 thread-safety invariant를 약화시켰습니다. 표준 C++ 메모리 semantics 하에서 JSC cell 내부의 unique_ptr 필드를 concurrent하게 수정하는 것은 undefined behavior입니다. m_regExpBytecode에 use-after-free 또는 torn write 조건이 발생할 수 있습니다. 수정은 두 가지 방식을 결합합니다. 하나는 직렬화를 위한 cellLock() 취득이고, 다른 하나는 compiler thread가 컴파일 자체를 시작하지 못하도록 막는 compile-time matchFrom gating입니다. gating 패턴이 더 근본적인 수정에 해당합니다. 직렬화에 그치지 않고, compiler thread 측에서 변경 자체를 제거하기 때문입니다. JSC 내에서 concurrent compiler thread가 접근할 수 있는 cell에 lazy-compilation 멤버가 있는 경우, "check then write" 패턴은 잠재적인 race가 됩니다.

Audit directions

JSC concurrent compiler thread에서 도달 가능한 cell 멤버의 lazy compilation/초기화. 최초 사용 시 채워지는 모든 cell 멤버(bytecode cache, sample-string cache, derived-structure cache 등)를 점검해야 합니다. matchConcurrently 방식의 경로나 DFG/FTL AbstractInterpreter constant folding을 통해 도달 가능한 멤버가 대상입니다. matchConcurrently, Yarr::MatchFrom::CompilerThread, JSCell 서브클래스의 XxxIfNecessary/compileIfNecessary/ensureXxx 계열 helper를 검색합니다. 각각에 대해 non-mutator 컨텍스트에서 해당 helper가 호출 가능한지, cellLock()을 취득하는지 확인해야 합니다.
동작을 compile time에 gating하는 MatchFrom 방식의 thread-context template parameter. MatchFrom과 같은 방식의 parameter를 사용하는 다른 JSC primitive(Yarr, JSGlobalObject lookup, structure transition 등)를 점검해야 합니다. template 함수 내부의 모든 변경 동작이 non-mutator instantiation에서 정적으로 제외되는지 확인합니다. 단순히 "런타임에 발생 가능성이 낮다"는 수준이 아니라, 정적으로 보장되어야 합니다.
동기화 없이 쓰이는 JSCell 서브클래스의 std::unique_ptr 타입 멤버. Source/JavaScriptCore/runtime과 Source/JavaScriptCore/yarr에서 cell 타입에 선언된 std::unique_ptr<...> m_ 멤버를 검색합니다. 각 쓰기 지점에서 cellLock()을 보유하고 있거나, mutator-only로 동작함이 보장되는지 확인해야 합니다.
parallel-compiler entry point를 조사해야 합니다. matchConcurrently가 compiler thread에서 호출 가능한 유일한 Yarr 경로인지 확인합니다. 또한 동일한 JIT-failure punt가 다른 위치(예: RegExpObject 작업)에도 존재하는지, 있다면 동등한 MatchFrom 방식의 gating이 적용되어 있는지 점검해야 합니다.