Bug 279994 – [GTK][WPE] REGRESSION(283749@main) Build broken because of pointer use after free

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Miguel Gomez]

I don't know why the bots are building properly and they are not showing this error, but this is what I'm getting when trying to build ToT:

In file included from /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/NeverDestroyed.h:32,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/platform/animation/TimingFunction.h:27,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/KeyframeInterpolation.h:30,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/BlendingKeyframes.h:28,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/BlendingKeyframes.cpp:23,
                 from /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WebCore/DerivedSources/unified-sources/UnifiedSource-a6b8b600-2.cpp:1:
In member function ‘bool WTF::RefCountedBase::derefAllowingPartiallyDestroyedBase() const’,
    inlined from ‘bool WTF::RefCountedBase::derefBase() const’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:155:51,
    inlined from ‘void WTF::RefCounted<T>::deref() const [with T = WebCore::CSSStyleValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:211:22,
    inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WebCore::CSSNumericValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/Ref.h:62:23,
    inlined from ‘WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::~RefPtr() [with T = WebCore::CSSNumericValue; _PtrTraits = WTF::RawPtrTraits<WebCore::CSSNumericValue>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WebCore::CSSNumericValue>]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefPtr.h:60:61,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:79:1,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:49:1:
/host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:141:33: error: pointer used after ‘static void WebCore::CSSStyleValue::operator delete(void*)’ [-Werror=use-after-free]
  141 |         unsigned tempRefCount = m_refCount - 1;
      |                                 ^~~~~~~~~~
In file included from /host/home/magomez/webkit/WebKit/Source/WebCore/css/typedom/CSSNumericValue.h:30,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.h:28,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/WebAnimationTypes.h:28,
                 from /host/home/magomez/webkit/WebKit/Source/WebCore/animation/KeyframeInterpolation.h:31:
In destructor ‘virtual WebCore::CSSStyleValue::~CSSStyleValue()’,
    inlined from ‘void WTF::RefCounted<T>::deref() const [with T = WebCore::CSSStyleValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:212:13,
    inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WebCore::CSSUnitValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/Ref.h:62:23,
    inlined from ‘WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::~RefPtr() [with T = WebCore::CSSUnitValue; _PtrTraits = WTF::RawPtrTraits<WebCore::CSSUnitValue>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WebCore::CSSUnitValue>]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefPtr.h:60:61,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:78:5,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:49:1:
/host/home/magomez/webkit/WebKit/Source/WebCore/css/typedom/CSSStyleValue.h:114:13: note: call to ‘static void WebCore::CSSStyleValue::operator delete(void*)’ here
  114 |     virtual ~CSSStyleValue() = default;
      |             ^
In member function ‘bool WTF::RefCountedBase::derefAllowingPartiallyDestroyedBase() const’,
    inlined from ‘bool WTF::RefCountedBase::derefBase() const’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:155:51,
    inlined from ‘void WTF::RefCounted<T>::deref() const [with T = WebCore::CSSStyleValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:211:22,
    inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WebCore::CSSNumericValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/Ref.h:62:23,
    inlined from ‘WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::~RefPtr() [with T = WebCore::CSSNumericValue; _PtrTraits = WTF::RawPtrTraits<WebCore::CSSNumericValue>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WebCore::CSSNumericValue>]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefPtr.h:60:61,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:79:1,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:49:1:
/host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:141:33: error: pointer used after ‘static void WebCore::CSSStyleValue::operator delete(void*)’ [-Werror=use-after-free]
  141 |         unsigned tempRefCount = m_refCount - 1;
      |                                 ^~~~~~~~~~
In destructor ‘virtual WebCore::CSSStyleValue::~CSSStyleValue()’,
    inlined from ‘void WTF::RefCounted<T>::deref() const [with T = WebCore::CSSStyleValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefCounted.h:212:13,
    inlined from ‘static void WTF::DefaultRefDerefTraits< <template-parameter-1-1> >::derefIfNotNull(T*) [with T = WebCore::CSSUnitValue]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/Ref.h:62:23,
    inlined from ‘WTF::RefPtr<T, <template-parameter-1-2>, <template-parameter-1-3> >::~RefPtr() [with T = WebCore::CSSUnitValue; _PtrTraits = WTF::RawPtrTraits<WebCore::CSSUnitValue>; _RefDerefTraits = WTF::DefaultRefDerefTraits<WebCore::CSSUnitValue>]’ at /host/home/magomez/webkit/WebKit/WebKitBuild/GTK/Release/WTF/Headers/wtf/RefPtr.h:60:61,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:78:5,
    inlined from ‘WebCore::CSSNumberishTime::CSSNumberishTime(WebCore::CSSNumberish)’ at /host/home/magomez/webkit/WebKit/Source/WebCore/animation/CSSNumberishTime.cpp:49:1:
/host/home/magomez/webkit/WebKit/Source/WebCore/css/typedom/CSSStyleValue.h:114:13: note: call to ‘static void WebCore::CSSStyleValue::operator delete(void*)’ here
  114 |     virtual ~CSSStyleValue() = default;
      |             ^
cc1plus: all warnings being treated as errors

[Michael Catanzaro]

You're surely using a different version of GCC than the bots?

Unfortunately this does not look like a false positive.

[Michael Catanzaro]

This was introduced in 273805@main. Looks like 283749@main was just enough for GCC to uncover it.

[Miguel Gomez]

(In reply to Michael Catanzaro from comment #1)
> You're surely using a different version of GCC than the bots?

I'm using gcc 12.3.0, but I'm not sure what version are the bots running.

[Miguel Gomez]

The bots are running gcc13, and that's probably the reason why they are not getting the build problem that I'm getting.

But we need to support gcc12 for a while still, so we need to fix this.

[Michael Catanzaro]

Well we need to fix it regardless of what compiler we are using if the diagnosis is correct and this is indeed a use after free.

If we can prove the diagnosis is incorrect, then we can just suppress the warning easily enough. But if we're wrong that just gives attackers a roadmap for how to find a use after free bug. (Though in fairness, they've surely already found this bug report, since it's public and uses sensitive keywords.)

[Carlos Alberto Lopez Perez]

The issue is that post-commit bots are not stopping the build on warnings.

Only the EWS bots (pre-commit) are stopping the build on warnings, but those use the flatpak build which much more modern GCC

I wonder why gcc-12 catches this but newer gcc doesn't?

[Michael Catanzaro]

The use after free warning in GCC 12 is just bad. Presumably GCC 13 toned down the false positives. (That said, this really doesn't look like a false positive? I'm not sure.)

Anyway, good news: I think this should be fixed by 284164@main.

[Michael Catanzaro]

So the commit message of 283685@main indicates that RefPtrAllowingPartiallyDestroyed is only used when a human has decided it's OK. I don't know whether GCC knows better than the human or not. Of course compilers are generally smarter than humans, but this particular warning really is half-baked. That said, this code also looks very risky.

Since the problem is surely fixed on the main branch, what I would do is suppress the warning only on the stable branch. (We could alternatively try backporting 283685@main and 284164@main, though I'm not sure whether that's a good idea.)

[Miguel Gomez]

I've just pulled and this is still happening for me.

[Miguel Gomez]

283749@main was reverted in main, so this is not happening anymore.