Branch: refs/heads/main
Home: https://github.com/WebKit/WebKit
Commit: f0d5be8210e3d8c5ac5fd6c86e86195b64cf7bb9
https://github.com/WebKit/WebKit/commit/f0d5be8210e3d8c5ac5fd6c86e86195b64cf7bb9
Author: Kiet Ho <[email protected]>
Date: 2026-08-28 (Fri, 28 Aug 2026)
Changed paths:
A LayoutTests/fast/canvas/canvas-filter-fillText-crash-expected.txt
A LayoutTests/fast/canvas/canvas-filter-fillText-crash.html
M Source/WebCore/SaferCPPExpectations/UncheckedLocalVarsCheckerExpectations
M Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp
Log Message:
-----------
CanvasRenderingContext2DBase::drawTextUnchecked: don't re-use pointer
returned by fontProxy()
https://bugs.webkit.org/show_bug.cgi?id=318372
rdar://175759731
Reviewed by Simon Fraser.
CanvasRenderingContext2D::fontProxy() returns the pointer to State::font
(a FontProxy) of the top State in the state stack. State stores the
FontProxy by value, so the FontProxy goes away when the State is deallocated.
This could happen when the state stack (a Vector<State, 1>) grows beyond
the storage buffer: a new buffer is created, existing State objects are
copied/moved to the new buffer and the old copies deallocated. Hence,
pointers returned by fontProxy() aren't safe to be re-used, because
between the time when the pointer is obtained and when it's used, some
operations might've manipulated the state stack and caused the FontProxy
to go away.
CanvasRenderingContext2DBase::drawTextUnchecked is one place where it happens:
it (1) holds on to the font cascade from the FontProxy returned by fontProxy(),
(2) creates a CanvasFilterContextSwitcher, whose constructor calls save() which
manipulates the state stack, then (3) uses the saved font cascade from the
FontProxy which might have been deallocated:
void CanvasRenderingContext2DBase::drawTextUnchecked(...)
{
auto& fontCascade = this->fontProxy()->fontCascade(); <-- (1)
[...]
auto targetSwitcher = CanvasFilterContextSwitcher::create(*this, textRect);
<-- (2)
[...]
auto drawText = [&](...) {
[...]
fontCascade.drawGlyphBuffer(...); <-- (3)
(actually, 317546@main indirectly fixes this by avoiding saving state when
creating CanvasFilterContextSwitcher. But as explained above, re-using
fontCascade
is unsafe, so this patch still has merits, even though it's not fixing anything)
Fix this by not holding onto pointers returned by fontProxy(). Instead, whenever
the FontProxy is needed, call fontProxy() so we're guaranteed to have a pointer
to a live FontProxy. Additionally, FontCascade can be made CheckedPtr, so wrap
it in CheckedPtr/CheckedRef whenever possible.
Future patches could improve on this by making FontProxy ref-counted, so the
pointer returned by fontProxy() is guaranteed to be alive no matter how the
State object storing it is copied/moved around.
Test: fast/canvas/canvas-filter-fillText-crash.html
* LayoutTests/fast/canvas/canvas-filter-fillText-crash-expected.txt: Added.
* LayoutTests/fast/canvas/canvas-filter-fillText-crash.html: Added.
* Source/WebCore/SaferCPPExpectations/UncheckedLocalVarsCheckerExpectations:
* Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::drawTextUnchecked):
Originally-landed-as: [email protected] (1d58c6a24867).
rdar://175759731
Canonical link: https://commits.webkit.org/320073@main
To unsubscribe from these emails, change your notification settings at
https://github.com/WebKit/WebKit/settings/notifications
