87194599a43b21533d276cae83d8f33d04d4cc02  layout: Change a DUMP_WILL_BE_NOTREACHED() to DCHECK()

The bug at a glance

The memory-safety guard against the use-after-free is the early return;, and that return already existed before this commit. What the patch actually changes is the assertion that fires when the guarded condition is reached: a release-build DUMP_WILL_BE_NOTREACHED() (which captures a crash dump and, on the migration path, is scheduled to become a fatal NOTREACHED()) is downgraded to a debug-only DCHECK. So the exploitable primitive - painting a LayoutBlock that still NeedsLayout() while its descendant LayoutObjects may have been freed - is never actually reached in either the old or new code, because both take the early return. The residual risk is (a) release-build stability: users hitting this path generated dumps and were at risk of a fatal upgrade of the soft-NOTREACHED, and (b) the underlying layout-lifecycle invariant violation (reaching paint with a dirty subtree) is a real bug that this commit papers over rather than root-causes. That combination - a genuine but already-mitigated UAF vector plus an availability regression - lands this at low / CVSS 4.3. It is not a memory-corruption fix; it is defensive assertion tuning on top of an existing guard.

The value here is not in the one-line diff but in what the guarded condition tells you. Reaching LayoutBlock::Paint() with NeedsLayout() true and !ChildLayoutBlockedByDisplayLock() means the paint walk was entered against a subtree that layout has already marked stale; the code comment states outright that descendants ‘maybe already destroyed’. That is the classic Blink shape of a paint-after-layout-teardown UAF. The interesting question for an auditor is which caller lets a dirty, non-display-locked block reach paint at all - the assertion is being kept precisely because that state is still occurring in the field (bug 478682594). The diff itself only decides how loudly to complain.

Root cause

In the Blink migration scheme, DUMP_WILL_BE_NOTREACHED() is a soft form of NOTREACHED(): in release builds it records a non-fatal crash dump and then falls through, and it is intended to be flipped to a hard, process-terminating NOTREACHED() once the offending path is believed eliminated. Here the branch body was { DUMP_WILL_BE_NOTREACHED(); return; }, so functionally the block was already skipped (never painted) - the safety property held. The problem was operational: real users kept hitting the path, so release builds produced dumps and were on track to start crashing once the NOTREACHED hardened. The patch hoists the predicate into a named is_dirty boolean, replaces the in-branch dump with a DCHECK(!is_dirty) placed before the branch, and keeps the if (is_dirty) [[unlikely]] return;. Net effect: debug and dev builds still assert loudly (surfacing the invariant break for developers), while release builds silently and safely take the early return with no dump and no risk of a fatal NOTREACHED. Memory safety is unchanged; crash-resilience improves.

Key code

LayoutBlock::Paint(): the release-fatal dump becomes a debug DCHECK, early return preserved

// Avoid painting dirty objects because descendants maybe already destroyed.
bool is_dirty = NeedsLayout() && !ChildLayoutBlockedByDisplayLock();
DCHECK(!is_dirty);
if (is_dirty) [[unlikely]] {
  return;
}

Patch walkthrough

• third_party/blink/renderer/core/layout/layout_block.cc — The guard in LayoutBlock::Paint() is rewritten. The condition NeedsLayout() && !ChildLayoutBlockedByDisplayLock() is extracted into a local is_dirty. The old DUMP_WILL_BE_NOTREACHED() inside the taken branch (release: emits a dump and continues to the return; on migration: would eventually terminate) is removed and replaced by DCHECK(!is_dirty) evaluated unconditionally before the branch. The if (is_dirty) [[unlikely]] { return; } early-out is retained verbatim, so the actual behaviour - refusing to paint a dirty, non-display-locked block - is identical. Only the assertion severity in release builds changes: from crash-dump/soon-fatal to silent-safe-return.

Background

DUMP_WILL_BE_NOTREACHED() — A Blink migration macro: a NOTREACHED() that is deliberately non-fatal in release for now (captures a crash dump, then falls through) so that a suspected-impossible path can be observed in the wild before being hardened into a terminating NOTREACHED().

DCHECK — Debug-only assertion. Compiled out of release/official builds, so it can flag an invariant violation for developers without affecting shipped-build stability.

NeedsLayout() — True when a LayoutObject’s subtree is marked dirty and must be re-laid-out before it is valid to paint or read geometry. Painting a NeedsLayout() subtree can touch stale or freed children.

ChildLayoutBlockedByDisplayLock() — content-visibility / display-lock state where child layout is intentionally deferred; in that case a ‘dirty’ block is legitimately not-yet-laid-out and must not be treated as the error condition, which is why it is excluded from is_dirty.

Vulnerability window

1. signal — Release-build crash dumps / DUMP_WILL_BE_NOTREACHED reports accumulate against bug 478682594, showing LayoutBlock::Paint() reached with a dirty, non-display-locked block.
2. mitigation — 2026-01-28: Kent Tamura downgrades the dump to a DCHECK to stop release-build crashes while keeping the safe early return and preserving developer-build visibility. Landed as #1576362.
3. open — Root cause (why a dirty block reaches paint) remains under investigation; the DCHECK stays as a tripwire for debug/dev builds.

Proof of concept

No PoC and none is warranted. The commit contains no test and does not remove a guard - it strengthens release-build resilience over an existing early return. Triggering the underlying invariant break requires driving layout into a state where a block is dirty and not display-locked at paint time; even then the early return prevents any memory access, so there is nothing to demonstrate as a memory-safety effect. Fabricating exploit code would misrepresent the change.

Exploitation

1. precondition — An attacker would need to force LayoutBlock::Paint() to run while NeedsLayout() is true and child layout is not display-locked - the exact state this guard detects.
2. blocked — Even in that state, both old and new code take the early return before any descendant is touched, so no freed memory is dereferenced. The guard is the mitigation; no primitive is reachable through this path.
3. residual — The only attacker-observable effect historically was a release crash dump (availability). That is what this commit removes.

Detection & hunting

For defenders and detection engineers:

• crash reports referencing DUMP_WILL_BE_NOTREACHED in LayoutBlock::Paint — Pre-patch builds emit dumps with this signature keyed to bug 478682594; a spike indicates a reproducible content pattern driving dirty blocks into paint.
• DCHECK(!is_dirty) failures in dev/canary — Post-patch, debug and Canary builds will assert on the same condition - route these to the layout lifecycle owners to find the real caller.

Audit directions

• Blink paint entry points — Search core/layout for other paint/read paths gated by NeedsLayout()/DUMP_WILL_BE_NOTREACHED and confirm each has a real early return, not just a dump-and-continue that could later dereference stale descendants.
• layout lifecycle -> paint boundary — Trace which callers can leave a LayoutBlock dirty and non-display-locked when PaintLayer painting begins; the durable fix is to prevent entry, not to assert on it.
• display-lock interactions — Review ChildLayoutBlockedByDisplayLock() exclusions across guards to ensure content-visibility state is consistently treated as non-error rather than being mistaken for a stale subtree.