f076e1adb5  FEMorphologySoftwareApplier is not thread safe

The bug at a glance

FEMorphologySoftwareApplier runs SVG feMorphology on multiple worker threads via ParallelJobs, and the shared PixelBuffer was only RefCounted (single-thread refcounting), a thread-safety defect flagged by WebKit’s NoUncountedMemberChecker. Concurrent refcount operations and shared mutable pixel storage across threads are a data-race class that can manifest as heap corruption or a use-after-free of the buffer, all reachable from untrusted SVG content in the web process. Because the diff is a substantial correctness refactor and no memory-safety PoC is included, medium is the honest rating.

feMorphology’s software path parallelizes by splitting the image into vertical blocks, with every worker sharing the same source and destination PixelBuffer. PixelBuffer was RefCounted, not ThreadSafeRefCounted, so refcount manipulation and buffer lifetime across worker threads were not safe. The fix makes PixelBuffer thread-safe-refcounted and gives each job private scratch buffers.

Root cause

FEMorphologySoftwareApplier::applyPlatform estimates an optimal thread count from image area and kernel size, then dispatches ParallelJobs<ApplyParameters> whose worker applyPlatformWorker calls applyPlatformGeneric over a Y-range. Before the patch, the per-job ApplyParameters held a raw const PaintingData* and startY/endY, and PaintingData held raw pointers const PixelBuffer* srcPixelBuffer and PixelBuffer* dstPixelBuffer shared by every job. This is exactly the pattern WebKit’s NoUncountedMemberChecker objected to: FEMorphologySoftwareApplier.h was listed in NoUncountedMemberCheckerExpectations because it embedded uncounted PixelBuffer pointers used across the parallel-job boundary.

The underlying object, WebCore::PixelBuffer, derived from RefCounted<PixelBuffer>. RefCounted uses non-atomic ref()/deref(); it is only safe to manipulate from one thread. As the applier is reworked (and as any code takes a Ref on a worker thread), concurrent ref/deref of a RefCounted PixelBuffer is a data race whose classic consequence is a corrupted refcount and premature deallocation (use-after-free) of the pixel storage. The patch changes PixelBuffer to derive from ThreadSafeRefCounted<PixelBuffer> (swapping the wtf/RefCounted.h include for wtf/ThreadSafeRefCounted.h), making ref/deref atomic so the buffer can be safely referenced by workers.

The applier is then restructured so jobs do not share mutable buffers. ApplyParameters now carries RefPtr<PixelBuffer> sourceBuffer and destinationBuffer plus IntSize sourceSize / IntRect destinationRect / type / radius, replacing the old PaintingData raw-pointer struct. applyPlatformWorker RELEASE_ASSERTs the buffers, takes Ref sourceBuffer/destinationBuffer, and calls the new applyPlatformGeneric(sourceBuffer, destinationBuffer, sourceSize, destinationRect, type, radius). The new templated FilterEffectSoftwareParallelApplier.h::applyPlatformParallel gives each non-zero job its own scratch source and destination PixelBuffers via sourceBuffer->createScratchPixelBuffer(…), memcpySpan-ing the relevant (overlap-padded) source rows in before execution and memcpySpan-ing each job’s destination rows back into the shared destination afterward. Overlap between blocks is handled by extraHeight (derived from the kernel height) via shiftYEdgeBy/shiftMaxYEdgeBy so morphology at block edges reads enough neighboring rows; the layout test gains a fuzzy meta tolerance for the boundary rounding this introduces.

Key code

The root thread-safety change: PixelBuffer becomes atomically refcounted (PixelBuffer.h).

-#include <wtf/RefCounted.h>
+#include <wtf/ThreadSafeRefCounted.h>

 // Type for holding pixel buffers data.
 // For functions that source pixel buffers, see PixelBufferSourceView.
-class PixelBuffer : public RefCounted<PixelBuffer> {
+class PixelBuffer : public ThreadSafeRefCounted<PixelBuffer> {
     WTF_MAKE_NONCOPYABLE(PixelBuffer);

Patch walkthrough

• Source/WebCore/platform/graphics/PixelBuffer.h — The core memory-safety change: PixelBuffer now derives from ThreadSafeRefCounted<PixelBuffer> instead of RefCounted<PixelBuffer> (include switched accordingly), making its refcount atomic so it can be shared/referenced across ParallelJobs worker threads without racing ref()/deref().
• Source/WebCore/platform/graphics/filters/software/FEMorphologySoftwareApplier.h — Deletes the raw-pointer PaintingData struct (which held const PixelBuffer*/PixelBuffer* and int width/height/radii) and replaces the per-job ApplyParameters with RefPtr<PixelBuffer> sourceBuffer/destinationBuffer, IntSize sourceSize, IntRect destinationRect, type and IntSize radius. Function signatures for applyPlatformGeneric/applyPlatform change to take buffers and geometry explicitly, and applyPlatform now returns bool.
• Source/WebCore/platform/graphics/filters/software/FEMorphologySoftwareApplier.cpp — Reworks applyPlatformGeneric to operate on explicit source/destination buffers and rects (writing at pixelArrayIndex(x, y - startY, sourceWidth) into the per-job destination), applyPlatformWorker to RELEASE_ASSERT and Ref the job’s buffers, and applyPlatform to build one ApplyParameters and delegate parallel dispatch to applyPlatformParallel, falling back to a single-threaded applyPlatformGeneric. The apply() entry point now holds the source as a RefPtr and forwards buffers/operator/radius.
• Source/WebCore/platform/graphics/filters/software/FilterEffectSoftwareParallelApplier.h — New reusable template applyPlatformParallel<ApplyParameters>: splits the image into jobs of blockHeight rows (rejecting when blockHeight <= extraHeight), allocates per-job scratch source/destination PixelBuffers via createScratchPixelBuffer for all but job 0, copies the padded source rows in with memcpySpan, executes ParallelJobs, then copies each job’s destination rows back into the shared destination buffer. Overlap padding uses extraHeight with shiftYEdgeBy/shiftMaxYEdgeBy.
• Source/WebCore/SaferCPPExpectations/NoUncountedMemberCheckerExpectations — Removes FEMorphologySoftwareApplier.h from the allowlist of files permitted to hold uncounted members, reflecting that the applier no longer stores raw PixelBuffer pointers across the parallel-job boundary.
• LayoutTests/svg/filters/feMorphology-radius-cases.svg — Adds a fuzzy meta tolerance (maxDifference=0-255; totalPixels=0-320) to absorb minor pixel differences at parallel block boundaries introduced by the scratch-buffer / overlap-padding rework.
• Source/WebCore/WebCore.xcodeproj/project.pbxproj — Registers the new FilterEffectSoftwareParallelApplier.h header in the Xcode project.

Background

feMorphology filter — An SVG filter primitive that erodes or dilates the input image within a rectangular kernel of radius (radiusX, radiusY). WebKit’s software implementation computes per-column extrema and slides a kernel across each row, an embarrassingly parallel operation over disjoint output rows, which is why it is threaded.

ParallelJobs — WTF’s ParallelJobs<T> spawns worker threads, each receiving a T parameter and running a worker function. FEMorphologySoftwareApplier splits the image into vertical blocks and hands each block to a worker; correctness requires that whatever the workers touch (here PixelBuffers) be safe to access concurrently.

RefCounted vs ThreadSafeRefCounted — RefCounted<T> uses non-atomic reference counting for objects confined to one thread; ThreadSafeRefCounted<T> uses atomic operations so ref()/deref() are safe from multiple threads. Sharing a RefCounted object across worker threads is a data race that can corrupt the count and free the object while still in use.

NoUncountedMemberChecker (SaferCPP) — A WebKit static analyzer that flags classes holding raw (uncounted) pointers to refcounted objects, a common source of use-after-free. Files with known, reviewed exceptions are listed in NoUncountedMemberCheckerExpectations; removing FEMorphologySoftwareApplier.h from that list signals the raw-pointer PaintingData design was eliminated.

Scratch buffers and block overlap — Morphology at a block boundary must read rows belonging to neighboring blocks (up to the kernel radius). The new applyPlatformParallel allocates each job a private scratch source (padded by extraHeight rows via shiftYEdgeBy/shiftMaxYEdgeBy) and a private destination, then stitches results back, so jobs never write the same PixelBuffer concurrently.

Vulnerability window

1. Parallel design — The software morphology applier was written to thread across vertical blocks using ParallelJobs, with all jobs referencing one shared source and one shared destination PixelBuffer via raw pointers in PaintingData.
2. Latent thread-safety defect — PixelBuffer used non-atomic RefCounted, and the raw shared pointers across the job boundary were carried as an explicit NoUncountedMemberChecker exception, leaving a data-race / lifetime hazard on the shared buffers.
3. Reported — Filed as webkit.org/b/308910 (rdar://168776587), ‘FEMorphologySoftwareApplier is not thread safe’, by Said Abou-Hallawa, reviewed by Mike Wyrzykowski.
4. Refactor — PixelBuffer promoted to ThreadSafeRefCounted; the applier reworked to give each job private scratch source/destination buffers (new FilterEffectSoftwareParallelApplier.h) with explicit overlap padding, removing the shared-mutable-buffer race.
5. Fix landed — Canonical 315239@main, originally shipped on safari-7624 as 305413.516; layout test gains a fuzzy tolerance for boundary pixel differences.

Triggering

No memory-safety PoC is included; the only test change adds a fuzzy-match tolerance to an existing render test. Trigger: render an SVG whose feMorphology filter operates on a sufficiently large image that applyPlatform selects more than one thread (optimalThreadNumber > 1, driven by widthheightkernelFactor exceeding the minimalArea threshold, with a non-degenerate radius), repeatedly, to exercise concurrent access to the shared RefCounted PixelBuffer on a pre-patch build. Expected effect is a data race surfacing as intermittent corruption/crash, not a deterministic controlled primitive.

Exploitation

1. Reachability — Untrusted web content can instantiate an SVG feMorphology filter of arbitrary size and radius, directly driving the software applier’s threading decision in the web (content) process.
2. Race window — With a large enough filtered region multiple ParallelJobs run concurrently; on a pre-patch build they share a non-thread-safe RefCounted PixelBuffer, so concurrent refcount operations (and any concurrent buffer access) race.
3. Corruption (data-race class) — A raced refcount can drop the PixelBuffer prematurely (use-after-free) or leave a corrupted count; the observable outcome is nondeterministic crashes or wrong pixels. This is a classic race, so timing/heap grooming would be required and reliability is inherently poor.
4. Honest assessment — The patch provides no exploit and the effect is crash/corruption-prone rather than a demonstrated write-what-where; treat this as thread-safety hardening of an attacker-reachable filter path rather than a proven exploitable primitive.

Detection & hunting

For defenders and SOC / detection engineers:

• Intermittent crashes in PixelBuffer refcounting under filters — Watch for non-deterministic web-process crashes with PixelBuffer deref / ThreadSafeRefCounted or FEMorphologySoftwareApplier worker frames on the stack, especially under heavy SVG filter rendering.
• ThreadSanitizer reports — Run feMorphology-heavy content under TSan; data races on the shared PixelBuffer refcount or bytes across ParallelJobs workers pinpoint the pre-patch defect.
• Large feMorphology filters — Content using feMorphology with large regions and non-trivial radius (enough to spawn multiple jobs) repeatedly is a behavioral indicator worth flagging when correlated with filter-thread crashes.

Audit directions

• Other software filter appliers — Audit sibling appliers still in NoUncountedMemberCheckerExpectations (e.g. FEConvolveMatrixSoftwareApplier.h, FELightingSoftwareApplier.h, FETurbulenceSoftwareApplier.h) for the same shared-RefCounted-buffer-across-ParallelJobs pattern and migrate them to the new applyPlatformParallel scratch-buffer model.
• PixelBuffer sharing across threads — Enumerate all code that passes PixelBuffer references into worker threads or GPU/IPC boundaries and confirm the now-ThreadSafeRefCounted lifetime assumptions hold end to end.
• applyPlatformParallel correctness — Review the new template’s block/overlap math (blockHeight <= extraHeight rejection, shiftYEdgeBy/shiftMaxYEdgeBy padding, scratch sizing and memcpySpan offsets) for off-by-one or OOB copy risks, since it now performs manual byte copies between buffers of differing sizes.
• Degenerate/edge radius handling — Verify the isDegenerate and radius clamping paths in apply() cannot produce sourceSize/destinationRect combinations that break the parallel splitter’s assumptions.