f88c5d9a0c4dee653373dd5fd4d0bd20c9f1706a  Race condition in Node::traverseToOpaqueRoot

The bug at a glance

The racy read happens on the GC marking thread every time a JS reference to a disconnected DOM node is scanned, so it is reachable from ordinary script that holds wrapper references while mutating the tree on the main thread; no special privilege is needed. The consequence is a genuine data race between the concurrent collector reading parent/shadow-host pointers and the main thread mutating those same pointers during insertion/removal, which is undefined behavior that can yield a torn or stale opaque root and mis-marked (prematurely collected) or corrupted objects. CVSS 7.0 (High) is consistent with a memory-safety-adjacent race that is script-reachable but nondeterministic and hard to steer.

WebKit’s garbage collector computes the “opaque root” of a DOM node to decide reachability, and for a disconnected node it did this by walking parent and shadow-host pointers with traverseToOpaqueRoot(). The problem: that walk runs on the concurrent GC marking thread while the main thread is simultaneously splicing the very pointers being walked. There was even a // FIXME: Possible race? sitting in Node::opaqueRoot() next to the disconnected-node path. The fix stops computing the root during marking entirely: each Node now caches an m_shadowIncludingRoot pointer that is maintained on the main thread during insertion and removal, so the collector just reads a single stable field instead of racing a live traversal.

Root cause

The vulnerable state is a Node that is not connected to a document. WebKit’s JS bindings implement opaqueRoot() so the collector can coalesce reachability of an entire DOM subtree to a single “opaque root” object. In the old Node::opaqueRoot(), connected nodes took a locked fast path returning treeScope().documentScope(), but disconnected nodes fell through to traverseToOpaqueRoot() with the comment // FIXME: Possible race?.

traverseToOpaqueRoot() walked node->parentOrShadowHostNode() in a loop up to the topmost ancestor and returned it wrapped in a WebCoreOpaqueRoot. This traversal executes on the concurrent marking thread as the collector visits the node’s JS wrapper. Meanwhile the main thread can be running DOM mutation (insertBefore, removeChild, shadow-host attach) which reassigns m_parentNode and shadow-host links on the same nodes. Reading a chain of pointers that another thread is rewriting with no synchronization is a data race: the walk can observe a half-updated chain, follow a stale pointer, or land on the wrong root, causing the collector to associate the wrapper with an incorrect opaque root and potentially mismark liveness.

The fix removes the traversal from the marking path. A new member Node* m_shadowIncludingRoot is added (initialized to this in the Node constructor) and opaqueRoot() becomes an inline one-liner return WebCoreOpaqueRoot { m_shadowIncludingRoot } — a single pointer read, no walk. The cached value is kept correct entirely on the main thread: Node::updateShadowIncludingRoot() is called from insertionSteps() and removingSteps(), setting m_shadowIncludingRoot from the parent’s cached value, or, for a ShadowRoot, from its host’s cached root. ShadowRoot::setHost() also updates it when a host is attached.

Because the root is now a stored field rather than a recomputed traversal, shadowIncludingRoot() and rootNode() become trivial accessors (return *m_shadowIncludingRoot), and the old Node::shadowIncludingRoot(), Node::opaqueRoot() out-of-line body, and Node::traverseToOpaqueRoot() are deleted. The change also lets isDescendantOf, isShadowIncludingDescendantOf, and isComposedTreeDescendantOf short-circuit to false when two nodes have different cached roots, avoiding pointless traversals.

Key code

opaqueRoot() stops traversing during marking; the root is cached and maintained on the main thread

// Node.h - before: out-of-line, walked the tree on the marking thread
// WebCoreOpaqueRoot opaqueRoot() const final;
// WebCoreOpaqueRoot NODELETE traverseToOpaqueRoot() const;
// after: single cached-pointer read
inline WebCoreOpaqueRoot opaqueRoot() const final { return WebCoreOpaqueRoot { m_shadowIncludingRoot }; }
Node* m_shadowIncludingRoot { nullptr };

// Node.cpp - deleted racy path:
// SUPPRESS_NODELETE WebCoreOpaqueRoot Node::opaqueRoot() const {
//     if (isConnected()) { Locker ...; return { &treeScope().documentScope() }; }
//     // FIXME: Possible race?
//     return traverseToOpaqueRoot();
// }

// Node.cpp - cache maintained on the main thread during tree changes:
ALWAYS_INLINE void Node::updateShadowIncludingRoot() {
    if (auto* parent = parentNode())
        m_shadowIncludingRoot = parent->m_shadowIncludingRoot;
    else if (auto* shadowRoot = dynamicDowncast<ShadowRoot>(this)) [[unlikely]] {
        auto* host = shadowRoot->host();
        auto* root = host ? host->m_shadowIncludingRoot : shadowRoot;
        shadowRoot->setShadowIncludingRoot(root);
        m_shadowIncludingRoot = root;
    } else
        m_shadowIncludingRoot = this;
    ASSERT(traverseToShadowIncludingRoot(this) == m_shadowIncludingRoot);
}
// called from Node::insertionSteps() and Node::removingSteps()

Patch walkthrough

• Source/WebCore/dom/Node.h — Adds the Node* m_shadowIncludingRoot { nullptr } member alongside m_treeScope, declares updateShadowIncludingRoot(), and rewrites shadowIncludingRoot() and opaqueRoot() as inline accessors over the cached pointer. Deletes the declarations of traverseToOpaqueRoot() and the out-of-line shadowIncludingRoot(), and pulls in WebCoreOpaqueRoot.h so the inline opaqueRoot() can construct the wrapper.
• Source/WebCore/dom/Node.cpp — Initializes m_shadowIncludingRoot(this) in the constructor and updates SameSizeAsNode accordingly. Adds updateShadowIncludingRoot(), which sets the cached root from the parent, or from a ShadowRoot’s host, or to this, guarded by an ASSERT comparing against a debug-only traverseToShadowIncludingRoot() walk. Calls it from insertionSteps() and removingSteps(). Deletes the racy opaqueRoot(), shadowIncludingRoot(), and traverseToOpaqueRoot(), and adds fast-path root-equality checks to the three descendant predicates.
• Source/WebCore/dom/Element.cpp — Defines ShadowRoot::setHost(Element*) inline here, setting m_host and computing m_shadowIncludingRoot from the host’s cached root (or this when detached), and switches addShadowRoot() to call the new pointer-taking overload so the cached root is populated at host-attach time.
• Source/WebCore/dom/ShadowRoot.h / ShadowRoot.cpp — Adds Node* m_shadowIncludingRoot to ShadowRoot with shadowIncludingRoot()/setShadowIncludingRoot() accessors and the inline setHost(Element*) declaration, initializes it to this in the constructor, and updates SameSizeAsShadowRoot. This lets a shadow tree cache a root that points through its host into the light tree.
• Source/WebCore/dom/ContainerNodeInlines.h / NodeInlines.h — ContainerNode::rootNode() and Node::rootNode() no longer call traverseToRootNode() on the non-tree-scope path; they return the cached *m_shadowIncludingRoot (downcast to ContainerNode where needed), eliminating the walk from these hot accessors too.
• LayoutTests/TestExpectations and several .cpp includes — Skips a debug-assertion-tripping WebVTT leak test and adds NodeInlines.h/ContainerNodeInlines.h includes to DocumentFragment, DocumentType, XMLDocument, SinkDocument, and MathMLUnknownElement so the newly-inlined accessors resolve. No security logic here.

Background

Opaque root — A JSC/WebCore GC concept: EventTarget::opaqueRoot() returns a single representative object for a subtree so the collector can treat the whole DOM tree as one reachability unit. Getting it wrong risks marking a live object dead (use-after-free) or keeping garbage alive.

Concurrent marking — JavaScriptCore marks the heap on a separate thread while (or interleaved with) the main thread runs. Any DOM state the marker reads while the main thread can mutate it must be synchronized; unsynchronized pointer walks over parentOrShadowHostNode() are exactly the hazard here.

Shadow-including root — Per the DOM spec’s shadow-including root concept, the topmost node reached by following parents and then crossing from a ShadowRoot to its host. The patch caches this value per node so predicates and the collector can read it in O(1).

insertionSteps / removingSteps — WebCore’s per-node hooks run on the main thread whenever a node enters or leaves a tree. They are the natural, race-free place to recompute m_shadowIncludingRoot, since parent/host links are finalized there.

Vulnerability window

1. Legacy design — Node::opaqueRoot() computed a disconnected node’s root lazily by walking parent/shadow-host pointers via traverseToOpaqueRoot(), with the connected path taking a tree-scope lock.
2. Known suspicion — A // FIXME: Possible race? was left in the code beside the traverseToOpaqueRoot() fallthrough, acknowledging the disconnected path was not thread-safe against concurrent marking.
3. Report — Tracked as webkit.org/b/310029 / rdar://172327427 as a race condition in Node::traverseToOpaqueRoot, read by the concurrent GC while the tree mutates.
4. Fix — Ryosuke Niwa introduced a cached m_shadowIncludingRoot maintained in updateShadowIncludingRoot() (from insertion/removal) and ShadowRoot::setHost(), making opaqueRoot() a single pointer read; the racy traversals were deleted.
5. Ship — Landed as 314670@main; branch-landed as 305413.522 on the safari-7624 branch (rdar://176062041). No new test, since the behavior is identical apart from the race and the race is hard to reproduce reliably.

Triggering

No proof of concept is reconstructable from the patch. The commit explicitly ships no new test because the only behavioral difference is the elimination of a nondeterministic data race that is hard to reproduce. Triggering it would require winning a race between the concurrent JSC marking thread reading a disconnected node’s parent/shadow-host chain and the main thread mutating that same chain, which is not reliably steerable from script and the patch contains no primitive to demonstrate it.

Exploitation

1. Set up a disconnected, mutating tree — Script holds JS wrapper references to nodes in a detached subtree (so isConnected() is false and the racy traverseToOpaqueRoot() path was taken) while repeatedly appending/removing children to keep parent and shadow-host pointers churning.
2. Race the collector — Drive allocation to provoke concurrent GC so the marking thread scans those wrappers and walks the pointer chain at the same instant the main thread rewrites it. Any exploit depends on the marker observing a torn/stale chain and deriving a wrong opaque root.
3. Convert to a memory-safety error (theoretical) — A wrong opaque root can cause the collector to associate a wrapper with the incorrect reachability unit, potentially freeing a still-referenced object. This is nondeterministic and the patch provides no primitive; treat exploitability as plausible-but-unproven.

Detection & hunting

For defenders and SOC / detection engineers:

• ThreadSanitizer on DOM mutation vs. GC marking —
• Crashes inside concurrent marking —
• Debug assertion parity —

Audit directions

• Other opaqueRoot()/marking-time traversals —
• Cache-maintenance completeness —
• Descendant-predicate short-circuits —