[4] Data race in Range::visitNodesInGCThread during GC

HighWebCore DOMRace

The garbage collector read a DOM node the main thread had just freed

ab48873

High. Garbage-collector marking thread가 main thread에서 방금 마지막 strong reference를 해제한 Node를 dereference할 수 있습니다. 살아있는 Range를 들고 DOM을 변경하는 어떤 script에서도 도달 가능한 memory unsafety입니다. Critical로 격상되지 않는 이유는 reliability 때문입니다. Window가 몇 개의 instruction 폭에 불과하고, 해제된 allocation을 제때 재사용하는 것도 보장되지 않습니다.

DOM Range 객체 — document.createRange()와 getSelection().getRangeAt()이 반환하는 그것 — 는 두 개의 endpoint를 가지며, document는 트리가 그 아래에서 변경될 때마다 이를 최신 상태로 유지합니다. 각 endpoint는 RangeBoundaryPoint로, container node를 Ref<Node>로 저장합니다. 이는 절대 null이 될 수 없는 reference-counted pointer로, 할당 시 새 pointer를 저장하고 기존 pointer를 release합니다. 한편 JSC의 collector는 전용 marking thread에서 heap을 추적하며, 이때 main thread는 계속 실행됩니다. WebCore는 JS 객체가 아닌 것들을 "opaque root"로 collector에 보고하여 wrapper가 살아남도록 합니다. 이 동작이 안전하려면, marking visitor가 읽는 데이터가 marking 도중 immutable하거나 lock으로 직렬화되어 있어야 합니다.

관전 포인트: 살아있는 Range를 들고 tight loop에서 DOM을 변경하는 페이지는, main thread가 방금 해제한 Node를 collector thread가 읽게 만들 수 있습니다.

m_start와 m_end를 변경하는 부분에 lock을 추가합니다.

이 data race를 안정적으로 테스트할 방법이 없어 새 테스트는 추가하지 않습니다.

Source/WebCore/dom/Range.h

Source/WebCore/dom/Range.cpp

Source/WebCore/dom/Document.cpp

Patch Details

이번 변경은 mutator/collector 경계 양쪽에서 lock을 획득하도록 하는 것이 핵심이며, 여기에 부수적인 annotation 변경과 그에 맞물린 iteration 수정이 함께 포함되어 있습니다.

WebCore::Range에 mutable Lock m_boundaryPointLock 멤버가 추가되었고, <wtf/Lock.h>와 <wtf/Locker.h> include도 함께 들어갔습니다. Write 쪽에서는 Range::setStart와 Range::setEnd에 Locker scope가 추가되었습니다 (각각 두 곳 — m_start.set(...) / m_end.set(...) 호출 부분 하나, treeOrder 비교 이후의 m_end = m_start / m_start = m_end 정규화 부분 하나). 같은 방식의 Locker가 Range::collapse, selectNodeContents, nodeChildrenChanged, nodeChildrenWillBeRemoved, nodeWillBeRemoved, textInserted, textRemoved, textNodesMerged, textNodeSplit에도 추가되었습니다. Read 쪽에서는, 기존에 존재하던 Range::visitNodesInGCThread가 — signature 자체는 변경되지 않았고 diff에는 context로만 등장합니다 — addWebCoreOpaqueRoot(visitor, m_start.container()) / m_end.container() 호출 전에 동일한 lock을 획득하도록 바뀌었습니다.

부수적인 변경으로는, Range.h에서 nodeChildrenChanged, textInserted, textRemoved의 선언에서 NODELETE annotation이 제거되었고, 이에 맞춰 Document::textInserted / Document::textRemoved의 m_ranges iteration 방식이 for (auto& range : m_ranges) range.get()...에서 for (Ref range : m_ranges) range->...로 바뀌었습니다. 이제 NODELETE가 아니게 된 호출 전 구간에서 각 Range가 ref로 보호되도록 하기 위함입니다.

Reference-counted pointer 필드를 동기화 없이 동시에 읽는 패턴입니다. 이 필드의 mutation은 pointer 자체를 재작성하는 동시에 기존 객체의 마지막 reference를 해제합니다.

Background

Live DOM Range. Script에서 생성되며 start와 end boundary point를 갖고, 트리가 변경될 때마다 Document가 최신 상태로 유지하는 객체입니다. Document::m_ranges는 attach된 live range들을 보관하며, text 삽입 같은 DOM mutation은 등록된 각 range를 호출하여 (Document::textInserted → Range::textInserted) boundary offset과 container를 조정할 수 있게 합니다.

RangeBoundaryPoint와 Ref<T>. RangeBoundaryPoint는 Range::m_start와 Range::m_end를 뒷받침하는 실제 storage로, container node를 Ref<Node>로 보관하며 offset과 child-before pointer를 함께 가집니다. Ref<T>는 절대 null이 될 수 없는 reference-counted smart pointer입니다. Ref에 값을 할당하면 새 pointer를 저장하고 기존 pointer를 release하며, 이것이 마지막 reference였다면 기존 객체가 destroy됩니다.

Concurrent marking과 opaque root. JSC의 collector는 marking 작업을 전용 thread에서 수행하며, 그동안 mutator — 즉 main thread — 는 계속해서 JavaScript와 DOM operation을 실행합니다. WebCore는 DOM 트리의 root node 같은 non-JS 객체를 addWebCoreOpaqueRoot(visitor, node)를 통해 collector에 보고합니다. Opaque root가 mark된 wrapper는 살아남게 됩니다.

Lock / Locker. WebKit의 mutex와 그 RAII scope guard입니다. Locker locker { lock }은 construction 시점에 lock을 획득하고 scope 종료 시 release합니다.

NODELETE. 객체 destruction을 유발해서는 안 되는 code path임을 표시하는, WebCore method 선언에 붙는 annotation입니다.

Analysis

Root cause는 Range::visitNodesInGCThread가 — 이름과 JSC::AbstractSlotVisitor& parameter로 볼 때 marking thread에서 실행됨을 알 수 있습니다 — m_start.container() / m_end.container()를 아무런 동기화 없이 읽는 반면, main thread는 DOM API와 Range API를 통해 같은 멤버들을 자유롭게 mutate했다는 데 있습니다.

  Main thread (mutator)              JSC marking thread
  ─────────────────────              ──────────────────
                                     load m_start.container()
                                       -> Node* N  ──┐
  m_start.set(newContainer, ...)                     │
    store new pointer                                │
    deref old  -> refcount 0                         │
    ~Node()  -> free(N)                               │
                                     addWebCoreOpaqueRoot(visitor, N)
                                       -> dereferences N  ◄─┘  UAF

이번 패치가 guard하게 된 모든 mutation 경로 — set(), setToBeforeContents(), setToAfterContents(), collapse()/setStart()/setEnd() 안의 m_end = m_start 복사 대입, 그리고 boundary 조정 helper인 boundaryNodeWillBeRemoved, boundaryNodeChildrenWillBeRemoved, boundaryTextInserted, boundaryTextRemoved, boundaryTextNodesMerged, boundaryTextNodesSplit — 는 non-atomic한 pointer 덮어쓰기를 수행하는 동시에 이전에 저장되어 있던 Node의 strong reference를 해제합니다. Ref<Node> 할당은 두 단계로 이루어집니다. 새 raw pointer를 저장하고, 기존 pointer를 deref()하는 것입니다. addWebCoreOpaqueRoot(visitor, m_start.container())를 실행하는 marking thread는 해제될 Node*를 load한 뒤, main thread의 deref()가 이미 destructor를 실행한 이후에 그 pointer를 사용하게 될 수 있습니다. 이는 boundary point가 마지막 strong reference를 쥐고 있던 경우에 정확히 문제가 됩니다. addWebCoreOpaqueRoot는 raw pointer 값을 단순 비교하는 게 아니라 node로부터 opaque root를 도출하기 때문에, 해제된 객체가 실제로 dereference됩니다.

같은 race에서 파생되는, 상대적으로 약한 두 번째 실패 양상도 있습니다. Marker가 mutation 이전의 m_start와 mutation 이후의 m_end처럼 서로 다른 시점의 boundary pair를 관찰하는 torn read가 발생할 수 있고, 이 경우 해당 marking increment에서 계산된 opaque-root set은 어떤 Range의 일관된 snapshot에도 대응하지 않게 됩니다.

Race 상태에 도달하는 것 자체는 전적으로 web content가 유발할 수 있습니다. 살아있는 Range를 들고 DOM을 변경하는 어떤 script든 write 쪽을 실행시키고, concurrent marking이 해당 Range를 방문할 때마다 read 쪽이 실행됩니다. 예상 가능한 trigger 시퀀스는 다음과 같습니다. (1) detached subtree의 node를 가리키는 boundary container를 갖는 대량의 Range 객체를 할당하여, RangeBoundaryPoint의 Ref<Node>가 해당 node들의 마지막 strong reference가 되는 상태를 노립니다 — RangeBoundaryPoint가 현실적으로 container의 마지막 strong reference를 쥘 수 있는지가 이 trigger의 핵심 전제이며, 실제로는 경험적으로 확인이 필요합니다. (2) allocation 압력을 가해 concurrent marking이 오래 지속되는 window를 만듭니다. (3) tight loop에서 range.setStart(otherNode, 0) / range.collapse() / range.selectNodeContents(other)를 반복 호출하거나, removeChild / splitText / text mutation을 수행하여 boundaryNodeWillBeRemoved와 boundaryTextNodesSplit이 container를 재작성하게 만듭니다. 매 iteration마다 Ref<Node>가 덮어써지고 기존 node가 release됩니다.

이 조건들로 controlled outcome을 실제로 만들어내려면 (a) 몇 개의 instruction 폭에 불과한 window에서 이겨야 하고, (b) marker가 읽기 전에 해제된 Node allocation을 attacker가 원하는 데이터로 재점유해야 하며, (c) 그 read가 즉시 fault를 유발하지 않고 살아남아야 합니다. 이 세 조건이 모두 성립한다면, marker가 attacker가 영향을 미친 값을 opaque root로 등록할 가능성이 있고, 이는 다시 어떤 wrapper가 collection에서 살아남는지를 흔들 수 있습니다. Torn-read variant에서는 또 다른 방향이 도출됩니다. 동기화되지 않은 read가 실제로는 동시에 존재한 적 없는 boundary pair를 관찰할 수 있기 때문에, collector가 어떤 boundary가 실제로 가리키는 subtree의 opaque root를 등록하지 못할 가능성이 있습니다. 그 opaque root가 유일한 keep-alive였다면, 해당 wrapper들이 premature하게 collect되어 Range가 부분적으로 collapse된 object graph를 가리키는 상태로 남을 가능성이 있습니다. 두 방향 모두 deterministic한 primitive가 아니라 race timing에 의존합니다. 패치된 code path 안에서 attacker-controlled write는 확인되지 않습니다.

발견 경로는 concurrent-GC visit callback에 대한 패턴 감사였을 가능성이 높습니다. Marking thread가 동기화 없이 읽는 WebCore state를 찾아나서는 방식으로, visitNodesInGCThread가 plain Ref<Node> 기반 boundary point 두 개를 그대로 읽는 것은 그 전형적인 사례에 해당합니다. Crash-report telemetry 경로와도 형태가 일치합니다. Marking thread에서 opaque-root 보고 도중 발생하는 산발적이고 재현 불가능한 fault는 정확히 이 race가 만들어내는 증상이며, 이런 report는 보통 targeted review로 이어집니다. 이 정도 폭의 race를 fuzzing으로 찾아냈을 가능성은 낮고, 패치에도 테스트가 추가되지 않았습니다.

이 vulnerability는 mutator와 concurrent collector 사이의 lifetime 계약을 깨뜨림으로써 WebContent process 내부의 memory safety를 약화시킵니다. 패치 이전에는 GC thread가 main-thread DOM 코드가 방금 마지막 strong reference를 해제한 Node를 dereference할 수 있었습니다. 여기서 흔들리는 security-model 전제는, concurrent marking visitor에서 도달 가능한 데이터가 marking 도중 immutable하거나 동기화로 보호되어야 한다는 것입니다. 패치 이전 Range::m_start/m_end는 둘 중 어느 조건도 만족하지 못했습니다. 이 race를 안정적으로 이길 수 있는 attacker라면 collector thread로부터 해제된 Node memory의 read를 얻어낼 수 있고, collector가 root로 취급하는 대상을 흔들 수도 있습니다. 유리한 heap 조건 하에서는 이것이 단순한 crash를 넘어, renderer의 controllable use-after-free로 가는 발판이 될 가능성이 있습니다.

Insight: Ref/RefPtr은 pointer slot 자체에 대해서는 아무런 보호도 제공하지 않습니다. Refcount의 atomicity는 count 값 자체를 보호할 뿐, field를 보호하지도, "새 pointer 저장"과 "기존 pointer deref" 사이의 순서를 보호하지도 않습니다. Marking thread에서 읽는 것이 legal한 WebCore 멤버는 사실상 shared mutable state인데, type system은 그런 멤버와 평범한 main-thread-only 멤버를 전혀 구분해주지 않습니다. 이 fix가 의도적으로 lock을 걸지 않은 부분도 눈여겨볼 만합니다. setStart/setEnd의 두 Locker scope 사이에 끼어 있는, plain main-thread accessor와 unlocked makeBoundaryPoint(m_start) read가 그것입니다. 이 비대칭은 main thread가 유일한 writer라는 전제 하에서만 안전한데, 이 전제는 patch에서 명시적으로 assert되지 않고 암묵적으로만 인코딩되어 있습니다.

Audit directions

• Concurrent collector가 동기화 없이 읽는 반면, mutator는 그 field를 재작성하면서 동시에 기존 객체의 마지막 reference를 해제하는 object field — smart-pointer type은 refcount가 atomic이라는 이유로 안전해 보이지만, pointer slot 자체는 atomic이 아닙니다. Narrow: WebCore에서 opaque-root 보고 호출(addWebCoreOpaqueRoot()과 JSC::AbstractSlotVisitor& / visitAdditionalChildren를 인자로 받는 함수를 검색하고, 각각에 대해 읽고 있는 Ref/RefPtr 멤버가 lock 없이 main-thread 코드에서 재할당될 수 있는지 확인해야 합니다. Range는 이제 guard되었지만, visit callback을 가진 다른 DOM bookkeeping type들(selection, highlight registry, observer registry)도 같은 점검을 받을 필요가 있습니다. Code review에서는, scope 안에 Locker가 없는 상태로 non-atomic Ref/RefPtr 멤버를 dereference하는 visit*/AbstractSlotVisitor 함수 body가 tell에 해당합니다. Wider: background thread(scrolling thread, media/AV callback thread, GPU worker thread, AXIsolatedTree builder)가 main-thread가 소유한 smart-pointer 멤버를 읽는 모든 지점 — off-thread에서 읽히는 WeakPtr slot, guard 없이 swap되는 std::unique_ptr 멤버도 포함됩니다. Code-search 패턴은 Atomic도 없고 guarded-by annotation도 없고 근처에 Lock도 없이 선언된 멤버이면서, 함수 이름이나 comment가 non-main thread를 지칭하는 경우입니다. Widest: reference-counted handle이 보호하는 것은 count뿐이며, slot은 절대 보호하지 못합니다. Owning-pointer field를 여러 thread에서 읽는 코드라면 각자의 동기화나 atomic handle type이 필요합니다 — 이 관점은 Blink의 concurrent marking에서의 TracedReference, interior mutability를 통해 mutate되는 plain field 안의 Rust Arc<T>, Java/Go의 concurrent-mark write barrier에도 그대로 적용할 수 있습니다. 언제나 "이 collector thread가 방금 load한 값의 마지막 reference를 누가 drop할 수 있는가?"를 물어야 합니다.

• Range에 대해 이 patch가 도입한 비대칭적 locking 체계가 실제로 완전한지 확인해야 합니다. 좁게 보면, Range::m_start/m_end의 모든 writer를 나열하고 각각이 m_boundaryPointLock scope 안에 있는지 확인하는 작업입니다. Diff에 등장하는 mutator들은 확인되지만, Range::updateFromSelection과 Range::updateRangeForParentlessNodeMovedToNewDocument는 Range.h에 선언되어 있을 뿐 diff에는 나타나지 않으므로, 이 두 함수부터 먼저 살펴봐야 합니다. Code review 시 눈여겨볼 신호는, Range.cpp 안에서 non-const RangeBoundaryPoint method를 호출하거나 m_start/m_end에 직접 대입하면서도 감싸는 block에 Locker가 보이지 않는 경우입니다. 조금 더 넓게 보면, "writer thread 하나, reader thread 하나, writer만 lock을 건다"는 설계는 WebCore 곳곳에 동일한 잠재적 결함을 남길 수 있습니다. mutable Lock을 가지고 있으면서 일부 accessor만 그 lock을 획득하는 클래스를 찾아, single-writer라는 가정이 코드 상에 명시적으로 assert되어 있는지 아니면 그저 우연히 성립하는 것인지 따져봐야 합니다. 가장 넓게 보면, 부분적 locking은 문서화되지 않은 thread-affinity 가정 위에서만 안전하며, 그 가정이 코드로 assert되어 있지 않다면 다음 리팩터링에서 소리 없이 깨질 수 있습니다. 부분적으로만 lock이 걸린 필드의 lock 없는 accessor마다 assertIsMainThread()에 준하는 장치(또는 Rust라면 Send/!Sync marker)가 함께 있는지 확인하고, 이것이 없다면 그 자체를 finding으로 취급해야 합니다.

• 이 patch의 annotation 제거 부분도 별도의 signal로 점검해야 합니다. Range::nodeChildrenChanged, Range::textInserted, Range::textRemoved에서 NODELETE가 제거되었고, 이를 보완하기 위해 Document::textInserted/textRemoved는 m_ranges를 for (Ref range : ...) 형태로 순회하도록 바뀌었습니다. 좁게 보면, Document.cpp와 WebCore의 다른 registry들에서 raw 또는 weak element 컬렉션(m_ranges, marker/observer/listener registry 등)을 순회하면서 NODELETE가 아닌 callee를 호출하는 loop를 찾는 작업입니다. Code review 시 눈여겨볼 신호는, registry에 대한 range-based for가 Ref 대신 auto&와 .get()을 사용하고, loop body에서 임의의 WebCore 코드를 실행할 수 있는 method를 호출하는 경우입니다. 조금 더 넓게 보면, "callee가 element를 변경하거나 파괴할 수 있는 상태에서 container를 순회하는" 패턴은 어떤 observer-notification loop에서든, 최적화 과정에서 사라진 copyToVector snapshot에서든, WeakHashSet에 대한 어떤 for에서든 동일하게 나타날 수 있습니다. 주목해야 할 형태는, container를 snapshot으로 뜨지도 않고 element에 대해 강한 참조로 보호하지도 않는 loop입니다. 가장 넓게 보면, notification loop는 callback이 진행되는 동안 컬렉션의 snapshot을 갖고 있거나, 각 element에 대한 strong reference를 갖고 있어야 합니다. 이는 DOM event dispatch, Node의 EventEmitter, Qt의 signal/slot emission, handler가 항목을 추가하거나 제거할 수 있는 상태에서 순회되는 Vec<Weak<T>> 등에 모두 적용되며, 이어서 던져야 할 질문은 "callee가 재진입하여 내가 순회 중인 container를 수정할 수 있는가"입니다.