226495 2021-06-01 04:57:15 -0700 [GStreamer] clang analysis: Unlocked access in ImageDecoderGStreamer.cpp 2021-06-16 01:16:03 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 aboya pnormand aperez bugs-noreply calvaris cgarcia ews-watchlist gustavo menard pnormand vjaquez oldest_to_newest 1765572 0 aboya 2021-06-01 04:57:15 -0700 In ImageDecoderGStreamer.cpp, line 384: if (!decoder.m_messageDispatched) { Locker locker { decoder.m_messageLock }; decoder.m_messageCondition.wait(decoder.m_messageLock); } m_messageDispatched is guarded by m_messageLock (see ImageDecoderGStreamer.h): bool m_messageDispatched WTF_GUARDED_BY_LOCK(m_messageLock) { false }; Yet the value is being checked before taking the lock. I don't know how it should work but it sure looks like it shouldn't be that way given those guard preconditions. 1769426 1 431335 pnormand 2021-06-14 09:26:29 -0700 Created attachment 431335 Patch 1769781 2 431335 aboya 2021-06-15 06:47:17 -0700 Comment on attachment 431335 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=431335&action=review > Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp:380 > + if (!decoder.m_messageDispatched) LGTM. Normally you use while() instead of if(), but it should be fine if the condition is only signalled when m_messageDispatched is set to true, rather than to any value. 1769859 3 pnormand 2021-06-15 10:27:16 -0700 (In reply to Alicia Boya García from comment #2) > Comment on attachment 431335 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=431335&action=review > > > Source/WebCore/platform/graphics/gstreamer/ImageDecoderGStreamer.cpp:380 > > + if (!decoder.m_messageDispatched) > > LGTM. Normally you use while() instead of if(), but it should be fine if the > condition is only signalled when m_messageDispatched is set to true, rather > than to any value. Right, I think there's no need for a while() indeed. The condition is notified at most one time. 1770107 4 ews-feeder 2021-06-16 01:16:00 -0700 Committed r278925 (238856@main): <https://commits.webkit.org/238856@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 431335. 431335 2021-06-14 09:26:29 -0700 2021-06-16 01:16:02 -0700 Patch bug-226495-20210614092627.patch text/plain 2562 pnormand U3VidmVyc2lvbiBSZXZpc2lvbjogMjc4ODIzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMzU4Y2EyMzBmMmZlMTZm MjExNjNlN2VmMGIxMmY2ZGU1MjFjOWVjNC4uNWVjYmJiZjYyY2U5ZTVkNzhjZmY2ODUxM2MzZDFh NjEzZjAzOTZlYiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDIxLTA2LTE0ICBQaGls aXBwZSBOb3JtYW5kICA8cG5vcm1hbmRAaWdhbGlhLmNvbT4KKworICAgICAgICBbR1N0cmVhbWVy XSBjbGFuZyBhbmFseXNpczogVW5sb2NrZWQgYWNjZXNzIGluIEltYWdlRGVjb2RlckdTdHJlYW1l ci5jcHAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIy NjQ5NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFJl bW92ZSB1bmxvY2tlZCBhY2Nlc3MgdG8gdGhlIHRoZSBtX21lc3NhZ2VEaXNwYXRjaGVkIGluc3Rh bmNlIHZhcmlhYmxlLiBBbHNvIHRoZXJlIGlzIG5vCisgICAgICAgIG5lZWQgdG8gd2FpdCBvbiB0 aGUgY29uZGl0aW9uIGlmIHRoZSBkaXNwYXRjaGluZyBoYXBwZW5zIHN5bmNocm9ub3VzbHkgaW4g dGhlIGN1cnJlbnQKKyAgICAgICAgdGhyZWFkLiBObyBuZWVkIHRvIG5vdGlmeSB0aGUgY29uZGl0 aW9uIGVpdGhlciBiZWZvcmUgZGlzcGF0Y2hpbmcsIHRoZSBvbmx5IGNhbGwgdG8gd2FpdCgpCisg ICAgICAgIGlzIGFmdGVyIHRoZSBhc3luY2hyb25vdXMgZGlzcGF0Y2ggaGFzIGJlZW4gc2NoZWR1 bGVkLgorCisgICAgICAgICogcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL0ltYWdlRGVjb2Rl ckdTdHJlYW1lci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpJbWFnZURlY29kZXJHU3RyZWFtZXI6 OklubmVyRGVjb2Rlcjo6cHJlcGFyZVBpcGVsaW5lKToKKwogMjAyMS0wNi0xMyAgQ2hyaXMgRHVt ZXogIDxjZHVtZXpAYXBwbGUuY29tPgogCiAgICAgICAgIFJlbGF4ICJwYXJlbnQgbXVzdCBiZSBh biBIVE1MRWxlbWVudCIgcmVzdHJpY3Rpb24gaW4gb3V0ZXJIVE1MIHNldHRlcgpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vZ3JhcGhpY3MvZ3N0cmVhbWVyL0ltYWdlRGVjb2Rl ckdTdHJlYW1lci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFt ZXIvSW1hZ2VEZWNvZGVyR1N0cmVhbWVyLmNwcAppbmRleCBhMTYxYzQ3YzUwZGI4Mjg0N2U2ZmYy ZDY4NTI4YmQ4Y2NjNzIyZDUxLi4zNTJlYzY2NjRjODliZjY5ZjIyMmUyZDFlYzNjZWMxNDJmYTM0 Y2M3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ncmFwaGljcy9nc3RyZWFt ZXIvSW1hZ2VEZWNvZGVyR1N0cmVhbWVyLmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9y bS9ncmFwaGljcy9nc3RyZWFtZXIvSW1hZ2VEZWNvZGVyR1N0cmVhbWVyLmNwcApAQCAtMzY1LDcg KzM2NSw2IEBAIHZvaWQgSW1hZ2VEZWNvZGVyR1N0cmVhbWVyOjpJbm5lckRlY29kZXI6OnByZXBh cmVQaXBlbGluZSgpCiAgICAgICAgIHsKICAgICAgICAgICAgIExvY2tlciBsb2NrZXIgeyBkZWNv ZGVyLm1fbWVzc2FnZUxvY2sgfTsKICAgICAgICAgICAgIGRlY29kZXIubV9tZXNzYWdlRGlzcGF0 Y2hlZCA9IGZhbHNlOwotICAgICAgICAgICAgZGVjb2Rlci5tX21lc3NhZ2VDb25kaXRpb24ubm90 aWZ5T25lKCk7CiAgICAgICAgIH0KICAgICAgICAgaWYgKCZkZWNvZGVyLm1fcnVuTG9vcCA9PSAm UnVuTG9vcDo6Y3VycmVudCgpKQogICAgICAgICAgICAgZGVjb2Rlci5oYW5kbGVNZXNzYWdlKG1l c3NhZ2UpOwpAQCAtMzc2LDEwICszNzUsMTEgQEAgdm9pZCBJbWFnZURlY29kZXJHU3RyZWFtZXI6 OklubmVyRGVjb2Rlcjo6cHJlcGFyZVBpcGVsaW5lKCkKICAgICAgICAgICAgICAgICBpZiAod2Vh a1RoaXMpCiAgICAgICAgICAgICAgICAgICAgIHdlYWtUaGlzLT5oYW5kbGVNZXNzYWdlKHByb3Rl Y3RlZE1lc3NhZ2UuZ2V0KCkpOwogICAgICAgICAgICAgfSk7Ci0gICAgICAgIH0KLSAgICAgICAg aWYgKCFkZWNvZGVyLm1fbWVzc2FnZURpc3BhdGNoZWQpIHsKLSAgICAgICAgICAgIExvY2tlciBs b2NrZXIgeyBkZWNvZGVyLm1fbWVzc2FnZUxvY2sgfTsKLSAgICAgICAgICAgIGRlY29kZXIubV9t ZXNzYWdlQ29uZGl0aW9uLndhaXQoZGVjb2Rlci5tX21lc3NhZ2VMb2NrKTsKKyAgICAgICAgICAg IHsKKyAgICAgICAgICAgICAgICBMb2NrZXIgbG9ja2VyIHsgZGVjb2Rlci5tX21lc3NhZ2VMb2Nr IH07CisgICAgICAgICAgICAgICAgaWYgKCFkZWNvZGVyLm1fbWVzc2FnZURpc3BhdGNoZWQpCisg ICAgICAgICAgICAgICAgICAgIGRlY29kZXIubV9tZXNzYWdlQ29uZGl0aW9uLndhaXQoZGVjb2Rl ci5tX21lc3NhZ2VMb2NrKTsKKyAgICAgICAgICAgIH0KICAgICAgICAgfQogICAgICAgICBnc3Rf bWVzc2FnZV91bnJlZihtZXNzYWdlKTsKICAgICAgICAgcmV0dXJuIEdTVF9CVVNfRFJPUDsK