1976782 - AddressSanitizer: heap-use-after-free [@ get_relaxed] with READ of size 4

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: asan_log.txt

Found with m-c 20250707-32587445152c (--enable-address-sanitizer)

This was found by visiting a live website with an ASan build.

STR:

• Launch browser and visit site

This issue was triggered by visiting http://earnlab.com/.

==132334==ERROR: AddressSanitizer: heap-use-after-free on address 0x5040001b1850 at pc 0x7fffdf136b9b bp 0x7fffb0c52f80 sp 0x7fffb0c52f78
READ of size 4 at 0x5040001b1850 thread T41
    #0 0x7fffdf136b9a in get_relaxed /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-atomic.hh:174:35
    #1 0x7fffdf136b9a in operator int /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-atomic.hh:170:32
    #2 0x7fffdf136b9a in get_relaxed /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-object.hh:148:37
    #3 0x7fffdf136b9a in hb_object_trace<hb_blob_t> /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-object.hh:231:3
    #4 0x7fffdf136b9a in hb_object_destroy<hb_blob_t> /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-object.hh:287:3
    #5 0x7fffdf136b9a in hb_blob_destroy /builds/worker/checkouts/gecko/gfx/harfbuzz/src/hb-blob.cc:264:8
    #6 0x7fffdf4ffaf9 in ~AutoTable /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.h:380:20
    #7 0x7fffdf4ffaf9 in gfxFontEntry::UnitsPerEm() /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:298:1
    #8 0x7fffdf46f0e7 in gfxFT2FontBase::gfxFT2FontBase(RefPtr<mozilla::gfx::UnscaledFontFreeType> const&, RefPtr<mozilla::gfx::SharedFTFace>&&, gfxFontEntry*, gfxFontStyle const*, int, bool) /builds/worker/checkouts/gecko/gfx/thebes/gfxFT2FontBase.cpp:36:7
    #9 0x7fffdf47b800 in gfxFontconfigFont /builds/worker/checkouts/gecko/gfx/thebes/gfxFcPlatformFontList.cpp:1324:7
    #10 0x7fffdf47b800 in gfxFontconfigFontEntry::CreateFontInstance(gfxFontStyle const*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFcPlatformFontList.cpp:947:26
    ...

0x5040001b1850 is located 0 bytes inside of 48-byte region [0x5040001b1850,0x5040001b1880)
freed by thread T35 here:
    #0 0x5555556ad716 in free /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
    #1 0x7fffdf4ffaf9 in ~AutoTable /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.h:380:20
    #2 0x7fffdf4ffaf9 in gfxFontEntry::UnitsPerEm() /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:298:1
    #3 0x7fffdf46f0e7 in gfxFT2FontBase::gfxFT2FontBase(RefPtr<mozilla::gfx::UnscaledFontFreeType> const&, RefPtr<mozilla::gfx::SharedFTFace>&&, gfxFontEntry*, gfxFontStyle const*, int, bool) /builds/worker/checkouts/gecko/gfx/thebes/gfxFT2FontBase.cpp:36:7
    #4 0x7fffdf47b800 in gfxFontconfigFont /builds/worker/checkouts/gecko/gfx/thebes/gfxFcPlatformFontList.cpp:1324:7
    #5 0x7fffdf47b800 in gfxFontconfigFontEntry::CreateFontInstance(gfxFontStyle const*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFcPlatformFontList.cpp:947:26
    #6 0x7fffdf4d36a7 in gfxFontEntry::FindOrMakeFont(gfxFontStyle const*, gfxCharacterMap*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:250:22
    #7 0x7fffdf5f0efd in gfxFontGroup::GetFontAt(unsigned int, unsigned int, bool*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2093:16
    #8 0x7fffdf5f3011 in gfxFontGroup::GetFirstValidFont(unsigned int, mozilla::StyleGenericFontFamily*, bool*) /builds/worker/checkouts/gecko/gfx/thebes/gfxTextRun.cpp:2330:12
    #9 0x7fffe219ba09 in mozilla::dom::CanvasRenderingContext2D::DrawOrMeasureText(nsTSubstring<char16_t> const&, float, float, mozilla::dom::Optional<double> const&, mozilla::dom::CanvasRenderingContext2D::TextDrawOperation, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/canvas/CanvasRenderingContext2D.cpp:5129:46
    #10 0x7fffe219d069 in mozilla::dom::CanvasRenderingContext2D::MeasureText(nsTSubstring<char16_t> const&, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/canvas/CanvasRenderingContext2D.cpp:4618:10
    ...

previously allocated by thread T35 here:
    #0 0x5555556adac9 in calloc /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:75:3
    #1 0x7fffdf1365ab in hb_calloc /builds/worker/checkouts/gecko/gfx/harfbuzz/src/hb-common.cc:1245:53
    #2 0x7fffdf1365ab in hb_object_create<hb_blob_t> /builds/worker/checkouts/gecko/gfx/harfbuzz/src/graph/../hb-object.hh:240:24
    #3 0x7fffdf1365ab in hb_blob_create_or_fail /builds/worker/checkouts/gecko/gfx/harfbuzz/src/hb-blob.cc:116:16
    #4 0x7fffdf1364eb in hb_blob_create /builds/worker/checkouts/gecko/gfx/harfbuzz/src/hb-blob.cc:82:21
    #5 0x7fffdf52b5b8 in gfxFontEntry::FontTableHashEntry::ShareTableAndGetBlob(nsTArray<unsigned char>&&, gfxFontEntry*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:474:11
    #6 0x7fffdf52bb85 in gfxFontEntry::ShareFontTableAndGetBlob(unsigned int, nsTArray<unsigned char>*) /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:550:17
    #7 0x7fffdf52c0d4 in gfxFontEntry::GetFontTable(unsigned int) /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:571:10
    #8 0x7fffdf4ff79c in AutoTable /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.h:378:27
    #9 0x7fffdf4ff79c in gfxFontEntry::UnitsPerEm() /builds/worker/checkouts/gecko/gfx/thebes/gfxFontEntry.cpp:270:13
    #10 0x7fffdf46f0e7 in gfxFT2FontBase::gfxFT2FontBase(RefPtr<mozilla::gfx::UnscaledFontFreeType> const&, RefPtr<mozilla::gfx::SharedFTFace>&&, gfxFontEntry*, gfxFontStyle const*, int, bool) /builds/worker/checkouts/gecko/gfx/thebes/gfxFT2FontBase.cpp:36:7
    ...

Thread T41 created by T0 (Web Content) here:
    #0 0x5555556931f1 in pthread_create /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:250:3
    #1 0x7ffff73dc419 in _PR_CreateThread /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:429:10
    #2 0x7ffff73ca65e in PR_CreateThread /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:496:10
    #3 0x7fffdca8ed01 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:615:20
    #4 0x7fffe5b5ee03 in mozilla::dom::WorkerThread::Create(mozilla::dom::WorkerThreadFriendKey const&) /builds/worker/checkouts/gecko/dom/workers/WorkerThread.cpp:97:7
    #5 0x7fffe5acc477 in mozilla::dom::workerinternals::RuntimeService::ScheduleWorker(mozilla::dom::WorkerPrivate&) /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:1408:37
    #6 0x7fffe5acb16d in mozilla::dom::workerinternals::RuntimeService::RegisterWorker(mozilla::dom::WorkerPrivate&) /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:1291:19
    #7 0x7fffe5b24e04 in mozilla::dom::WorkerPrivate::Constructor(JSContext*, nsTSubstring<char16_t> const&, bool, mozilla::dom::WorkerKind, mozilla::dom::RequestCredentials, mozilla::dom::WorkerType, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, mozilla::dom::WorkerLoadInfo*, mozilla::ErrorResult&, nsTString<char16_t>, std::function<void (bool)>&&, std::function<void ()>&&, mozilla::ipc::Endpoint<mozilla::dom::PRemoteWorkerNonLifeCycleOpControllerChild>&&) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3190:24
    #8 0x7fffe5ae4847 in mozilla::dom::Worker::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::TrustedScriptURLOrUSVString const&, mozilla::dom::WorkerOptions const&, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/workers/Worker.cpp:80:41
    #9 0x7fffe182c326 in mozilla::dom::Worker_Binding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dom/bindings/./WorkerBinding.cpp:1084:52
    #10 0x7fffe864d575 in CallJSNative /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:501:13
    ...

Thread T35 created by T0 (Web Content) here:
    #0 0x5555556931f1 in pthread_create /builds/worker/fetches/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:250:3
    #1 0x7ffff73dc419 in _PR_CreateThread /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:429:10
    #2 0x7ffff73ca65e in PR_CreateThread /builds/worker/checkouts/gecko/nsprpub/pr/src/pthreads/ptthread.c:496:10
    #3 0x7fffdca8ed01 in nsThread::Init(nsTSubstring<char> const&) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:615:20
    #4 0x7fffe5b5ee03 in mozilla::dom::WorkerThread::Create(mozilla::dom::WorkerThreadFriendKey const&) /builds/worker/checkouts/gecko/dom/workers/WorkerThread.cpp:97:7
    #5 0x7fffe5acc477 in mozilla::dom::workerinternals::RuntimeService::ScheduleWorker(mozilla::dom::WorkerPrivate&) /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:1408:37
    #6 0x7fffe5acb16d in mozilla::dom::workerinternals::RuntimeService::RegisterWorker(mozilla::dom::WorkerPrivate&) /builds/worker/checkouts/gecko/dom/workers/RuntimeService.cpp:1291:19
    #7 0x7fffe5b24e04 in mozilla::dom::WorkerPrivate::Constructor(JSContext*, nsTSubstring<char16_t> const&, bool, mozilla::dom::WorkerKind, mozilla::dom::RequestCredentials, mozilla::dom::WorkerType, nsTSubstring<char16_t> const&, nsTSubstring<char> const&, mozilla::dom::WorkerLoadInfo*, mozilla::ErrorResult&, nsTString<char16_t>, std::function<void (bool)>&&, std::function<void ()>&&, mozilla::ipc::Endpoint<mozilla::dom::PRemoteWorkerNonLifeCycleOpControllerChild>&&) /builds/worker/checkouts/gecko/dom/workers/WorkerPrivate.cpp:3190:24
    #8 0x7fffe5ae4847 in mozilla::dom::Worker::Constructor(mozilla::dom::GlobalObject const&, mozilla::dom::TrustedScriptURLOrUSVString const&, mozilla::dom::WorkerOptions const&, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/workers/Worker.cpp:80:41
    #9 0x7fffe182c326 in mozilla::dom::Worker_Binding::_constructor(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dom/bindings/./WorkerBinding.cpp:1084:52
    #10 0x7fffe864d575 in CallJSNative /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:501:13
    ...

Comment 1

[Daniel Holbert [:dholbert]]

Tyson: would you mind capturing/sharing a recording of the crash, with rr/pernosco?

(I just tested locally with my own local asan debug+opt build, and was unable to repro any issues when visiting this site.)

I don't really know our font code, but I suspect it'd be be great to have MOZ_LOG logging enabled as well, as breadcrumbs during the pernosco recording and possible clues just before the crash. You you can add that by setting this env variable in the terminal where you capture the recording:

export MOZ_LOG="timestamp,sync,fontinit:5,fontlist:5,userfonts:5,fontInfoLog:5"

(pernosco has other ways of visualizing MOZ_LOG logging even if it's off, but it's often nicer to just have it there up-front.)

Thanks!

Comment 2

[Jonathan Kew [:jfkthame]]

Yes, if we can get a pernosco trace, that may help us understand the root cause here. Both this and the "see also" bug 1978492 look like they're related to ownership & lifetimes of font table data; we're apparently mismanaging this somehow when multiple threads are using the same font resource, and one thread ends up releasing a table while the other is still using it.

(It's possible that enabling logging will disturb timing such that the issue doesn't reproduce; if so, a trace without extra logging would still be very welcome.)

Comment 3

[Jonathan Kew [:jfkthame]]

I suspect the root of this (and probably other similar issues we've seen recently) may be a race involving gfxFontEntry::GetFontTable and its management of the font table cache.

We start by checking if there's an existing table in the cache, and if so just return a new reference to that blob. GetExistingFontTable takes a read-lock while checking the cache, so should be fine.

If GetExistingFontTable returned false, we use CopyFontTable to load the table data into an array, and then pass that to ShareFontTableAndGetBlob to record it in the cache and return a reference. We take a write-lock within ShareFontTableAndGetBlob so that we won't corrupt the hashtable via racing updates.

But that's not enough: the risk I see is that two GetFontTable requests for the same not-already-cached table will both fail to find it in the cache; they'll both load copies of the data using CopyFontTable, and then they'll both call ShareFontTableAndGetBlob. One of them will win the race to take the write-lock there, insert its entry into the cache, and return a reference to its caller. Then the other thread's ShareFontTableAndGetBlob proceeds, and overwrites the cache entry with its own copy of the (same) table data; but this means it returns a reference to a new blob with one owner, not an additional reference to the one that was created by the other thread. So we have two blobs that both think they're being managed by the same hashtable entry. Oops.

So ShareFontTableAndGetBlob needs to double-check that the table is still not present in cache after it has claimed the write lock. If it finds that it's present, this thread must have lost a race, and should just return a reference to the existing entry, not overwrite it.

(Without a pernosco trace to examine, I can't be certain this is the scenario that's happened here, but inspection of the code convinces me it'd be a realistic possibility, though probably pretty hard to hit with any reliability due to the short window of vulnerability between the call to GetExistingFontTable checking the cache, and ShareFontTableAndGetBlob taking the write-lock to update it.)

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: (secure)

Comment 5

[Jonathan Kew [:jfkthame]]

Although we don't have STR to reproduce/confirm this, we should assume it affects release/beta/esr as well as Nightly; this is code that's been around in basically this form for quite a while.

Comment 6

[Tyson Smith [:tsmith]]

(In reply to Daniel Holbert [:dholbert] from comment #1)

Tyson: would you mind capturing/sharing a recording of the crash, with rr/pernosco?

I have been unable to reproduce the issue myself. I have placed watches on the buckets to watch for more reports. I don't know if I will be able to get a Pernosco session in a reasonable amount of time. That said it looks like Jonathan has a fix. Once this lands I will be sure to update the bug if new reports come in with the patch applied.

Comment 7

[Jonathan Kew [:jfkthame]]

Comment on attachment 9502980 [details]
(secure)

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Probably not easily; it's clear we're preventing a possible race, but unclear how to trigger it or how an attacker could take advantage of it
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
• Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: (patch should transplant cleanly)
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely; just adds a check in case we lost a race with another thread.
• Is the patch ready to land after security approval is given?: Yes
• Is Android affected?: Yes

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9502980 [details]
(secure)

Approved to land and request uplift

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3887ea7eed9a
https://hg.mozilla.org/integration/autoland/rev/1cb92eab949f
Guard against possible race via gfxFontEntry::GetFontTable. r=gfx-reviewers,lsalzman

Comment 10

[Ryan VanderMeulen [:RyanVM]]

FYI, this'll need rebasing for ESR128/ESR115.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1cb92eab949f

Comment 12

[Jonathan Kew [:jfkthame]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D258728

Comment 13

[Phabricator Automation]

firefox-beta Uplift Approval Request

• User impact if declined: race condition managing font data, potentially resulting in crash or maybe other issues
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: (no known STR for testing)
• Risk associated with taking this patch: low
• Explanation of risk level: simple added check for an existing table entry
• String changes made/needed: none
• Is Android affected?: yes

Comment 14

[Jonathan Kew [:jfkthame]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D258728

Comment 15

[Phabricator Automation]

firefox-esr140 Uplift Approval Request

• User impact if declined: (see above)
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: (see above)
• Risk associated with taking this patch: (see above)
• Explanation of risk level: (see above)
• String changes made/needed: (see above)
• Is Android affected?: yes

Comment 16

[Jonathan Kew [:jfkthame]]

Attachment: (secure)

Comment 17

[Phabricator Automation]

firefox-esr128 Uplift Approval Request

• User impact if declined: (see comment 13 above)
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: (see above)
• Risk associated with taking this patch: (see above)
• Explanation of risk level: (see above)
• String changes made/needed: (see above)
• Is Android affected?: no

Comment 18

[Jonathan Kew [:jfkthame]]

Attachment: (secure)

Original Revision: https://phabricator.services.mozilla.com/D259122

Comment 19

[Phabricator Automation]

firefox-esr115 Uplift Approval Request

• User impact if declined: (see above)
• Code covered by automated testing: no
• Fix verified in Nightly: no
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: (see above)
• Risk associated with taking this patch: (see above)
• Explanation of risk level: (see above)
• String changes made/needed: (see above)
• Is Android affected?: yes

Comment 20

[Pulsebot]

https://github.com/mozilla-firefox/firefox/commit/8ddf0b6a858b
https://hg.mozilla.org/releases/mozilla-beta/rev/f558b6820777

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/f5b525f4d4b1

Comment 22

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/71f2b55b6c98

Comment 23

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr140/rev/c3fd840fa3bd

Comment 26

[BugBot [:suhaib / :marco/ :calixte]]

Copying crash signatures from duplicate bugs.