Closed Bug 1463884 Opened 3 years ago Closed 3 years ago

Assertion failure: NS_IsMainThread() || sServoFFILock->LockedForWritingByCurrentThread()

Categories

(Core :: Layout, defect, P2)

Unspecified
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla62
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 61+ fixed
firefox60 --- wontfix
firefox61 + fixed
firefox62 + fixed

People

(Reporter: jya, Assigned: jfkthame)

References

Details

(Keywords: csectype-race, regression, sec-low, Whiteboard: [adv-main61+][adv-esr60.1+])

Attachments

(3 files, 1 obsolete file)

Using current central on Linux x86_64:

at the start I get an assertion:

Assertion failure: NS_IsMainThread() || sServoFFILock->LockedForWritingByCurrentThread(), at /home/jyavenard/Work/Mozilla/mozilla-central/layout/style/ServoBindings.cpp:126
#01: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x5999e60]
#02: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10ea096]
#03: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10e9fd6]
#04: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b96efe]
#05: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b970c9]
#06: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0efb]
#07: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0c18]
#08: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2af3cb9]
#09: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x26e05bc]
#10: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10423b5]
#11: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1069cbc]
#12: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1a51207]
#13: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198be25]
#14: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bda5]
#15: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bd7d]
#16: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x103fd18]
#17: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libnspr4.so +0x42a05]
#18: ???[/lib/x86_64-linux-gnu/libpthread.so.0 +0x76db]
#19: clone[/lib/x86_64-linux-gnu/libc.so.6 +0x12188f]
#20: ??? (???:???)

Program /home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/firefox (pid = 53999) received signal 11.
Stack:
Keywords: regression
OS: Unspecified → Linux
What's the stack?
Flags: needinfo?(jyavenard)
The stack displayed in the console is what I get. There are no symbols resolved.

Likely related, E10s is deactivated. 300s after the assertion occurs, Firefox will close.
Flags: needinfo?(jyavenard)
Could you attach gdb or something and try to get a stack with symbols? I'm pretty concerned (and surprised to some extent) about this assertion firing, but I can't repro at all on https://hg.mozilla.org/mozilla-central/rev/47e81ea1ef10189ef210867934bf36e14cf223dc with e10s either enabled or disabled.

Do you happen to have of any particular user style / font-related prefs / whatnot?
Flags: needinfo?(jyavenard)
I run central in a debugger, the assertion is showing up in the console, it doesn't trigger gdb and my current debugging session, it's like a new process is spawned anyway, even with e10s disabled.
The console displays a message that things will shut-down in 300. It then continues to run for a little while and everything ends 300s later

It's ubuntu recently updated to 18.04
Flags: needinfo?(jyavenard)
Priority: -- → P2
Ok...

To reproduce the bug I do:
./mach run -P Development
where Development is a profile that has browser.tabs.remote.autostart set to false (disabling e10s)

I then go into the Preferences -> boom you see in the launching terminal:
Assertion failure: NS_IsMainThread() || sServoFFILock->LockedForWritingByCurrentThread(), at /home/jyavenard/Work/Mozilla/mozilla-central/layout/style/ServoBindings.cpp:126
#01: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x5999e60]
#02: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10ea096]
#03: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10e9fd6]
#04: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b96efe]
#05: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b970c9]
#06: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0efb]
#07: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0c18]
#08: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2af3cb9]
#09: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x26e05bc]
#10: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10423b5]
#11: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1069cbc]
#12: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1a51207]
#13: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198be25]
#14: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bda5]
#15: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bd7d]
#16: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x103fd18]
#17: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libnspr4.so +0x42a05]
#18: ???[/lib/x86_64-linux-gnu/libpthread.so.0 +0x76db]
#19: clone[/lib/x86_64-linux-gnu/libc.so.6 +0x12188f]
#20: ??? (???:???)

Program /home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/firefox (pid = 60787) received signal 11.
Stack:
#01: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x901309b]
#02: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x8fc861d]
#03: ???[/lib/x86_64-linux-gnu/libpthread.so.0 +0x12890]
#04: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x5999e6e]
#05: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10ea096]
#06: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10e9fd6]
#07: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b96efe]
#08: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2b970c9]
#09: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0efb]
#10: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2ad0c18]
#11: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x2af3cb9]
#12: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x26e05bc]
#13: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x10423b5]
#14: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1069cbc]
#15: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x1a51207]
#16: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198be25]
#17: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bda5]
#18: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x198bd7d]
#19: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libxul.so +0x103fd18]
#20: ???[/home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/libnspr4.so +0x42a05]
#21: ???[/lib/x86_64-linux-gnu/libpthread.so.0 +0x76db]
#22: clone[/lib/x86_64-linux-gnu/libc.so.6 +0x12188f]
#23: ??? (???:???)
Sleeping for 300 seconds.

during this time, I can perfectly run FF just fine, it is then killed after 300s with the message "sleeping done" being displayed.

There's a message like:
Type 'gdb /home/jyavenard/Work/Mozilla/obj-ff-dbg/dist/bin/firefox 60787' to attach your debugger to this thread.

which I did, but the backtrace showing in that gdb session is for the main process, and nothing show about ServoBindings.cpp
ah got one:

#0  0x00007fffe2b36e6e in AssertIsMainThreadOrServoLangFontPrefsCacheLocked() () at /home/jyavenard/Work/Mozilla/mozilla-central/layout/style/ServoBindings.cpp:126
#1  0x00007fffde287096 in nsLanguageAtomService::GetLanguageGroup(nsAtom*, bool*) (this=0x7fffdaa1c430, aLanguage=0x7fffb4757be0, aNeedsToCache=0x0) at /home/jyavenard/Work/Mozilla/mozilla-central/intl/locale/nsLanguageAtomService.cpp:99
#2  0x00007fffde286fd6 in nsLanguageAtomService::LookupLanguage(nsTSubstring<char> const&) (this=0x7fffdaa1c430, aLanguage=...) at /home/jyavenard/Work/Mozilla/mozilla-central/intl/locale/nsLanguageAtomService.cpp:46
#3  0x00007fffdfd33efe in gfxPlatformFontList::TryLangForGroup(nsTSubstring<char> const&, nsAtom*, nsTSubstring<char>&) (this=0x7fffdb974800, aOSLang=..., aLangGroup=0x7fffe83df50c <mozilla::detail::gGkAtoms+64332>, aFcLang=...) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/thebes/gfxPlatformFontList.cpp:1510
#4  0x00007fffdfd340c9 in gfxPlatformFontList::GetSampleLangForGroup(nsAtom*, nsTSubstring<char>&, bool) (this=0x7fffdb974800, aLanguage=0x7fffe83df50c <mozilla::detail::gGkAtoms+64332>, aLangStr=..., aCheckEnvironment=true) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/thebes/gfxPlatformFontList.cpp:1554
#5  0x00007fffdfc6defb in GetSystemFontList(nsTArray<nsTString<char16_t> >&, nsAtom*) (aListOfFonts=..., aLangGroup=0x7fffe83df50c <mozilla::detail::gGkAtoms+64332>) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/thebes/gfxFcPlatformFontList.cpp:1762
#6  0x00007fffdfc6dc18 in gfxFcPlatformFontList::GetFontList(nsAtom*, nsTSubstring<char> const&, nsTArray<nsTString<char16_t> >&) (this=0x7fffdb974800, aLangGroup=0x7fffe83df50c <mozilla::detail::gGkAtoms+64332>, aGenericFamily=..., aListOfFonts=...) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/thebes/gfxFcPlatformFontList.cpp:1800
#7  0x00007fffdfc90cb9 in gfxPlatformGtk::GetFontList(nsAtom*, nsTSubstring<char> const&, nsTArray<nsTString<char16_t> >&) (this=0x7fffdaa48d00, aLangGroup=0x7fffe83df50c <mozilla::detail::gGkAtoms+64332>, aGenericFamily=..., aListOfFonts=...) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/thebes/gfxPlatformGtk.cpp:180
#8  0x00007fffdf87d5bc in EnumerateFontsTask::Run() (this=0x7fffb3f48660) at /home/jyavenard/Work/Mozilla/mozilla-central/gfx/src/nsThebesFontEnumerator.cpp:152
#9  0x00007fffde1df3b5 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7fffc2dcce20, aMayWait=true, aResult=0x7fffaf7fbc3e) at /home/jyavenard/Work/Mozilla/mozilla-central/xpcom/threads/nsThread.cpp:1090
#10 0x00007fffde206cbc in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7fffc2dcce20, aMayWait=true) at /home/jyavenard/Work/Mozilla/mozilla-central/xpcom/threads/nsThreadUtils.cpp:519
#11 0x00007fffdebee207 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (this=0x7fffba4f3c00, aDelegate=0x7fffb3a64c80) at /home/jyavenard/Work/Mozilla/mozilla-central/ipc/glue/MessagePump.cpp:364
#12 0x00007fffdeb28e25 in MessageLoop::RunInternal() (this=0x7fffb3a64c80) at /home/jyavenard/Work/Mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:326
#13 0x00007fffdeb28da5 in MessageLoop::RunHandler() (this=0x7fffb3a64c80) at /home/jyavenard/Work/Mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:319
#14 0x00007fffdeb28d7d in MessageLoop::Run() (this=0x7fffb3a64c80) at /home/jyavenard/Work/Mozilla/mozilla-central/ipc/chromium/src/base/message_loop.cc:299
#15 0x00007fffde1dcd18 in nsThread::ThreadFunc(void*) (aArg=0x7fffffff0288) at /home/jyavenard/Work/Mozilla/mozilla-central/xpcom/threads/nsThread.cpp:425
#16 0x00007ffff67eba05 in _pt_root (arg=0x7fffb1464ca0) at /home/jyavenard/Work/Mozilla/mozilla-central/nsprpub/pr/src/pthreads/ptthread.c:201
#17 0x00007ffff7bbd6db in start_thread (arg=0x7fffaf7fc700) at pthread_create.c:463
#18 0x00007ffff6d9e88f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
this is from the "FontEnumThread"
I have no idea what that thread is, it's definitely not related to style, but whoever is doing that is doing something pretty unsafe... Jonathan, any ideas about how to fix this? This is unsafe.
Blocks: 1375978
Group: core-security
This is only related to about:preferences at least, which is good.
Flags: needinfo?(jfkthame)
Yeah, this arose because when Preferences was reorganized such that the Fonts menu appears on the main (default) prefs page, there was a concern that enumerating all the available fonts was taking a long time and blocking the initial display of the page.

So in bug 1375978, the enumeration of the fonts was moved to a separate thread, so that it doesn't block display of the prefs page (we just populate the Fonts menu later, once the list is ready).

On macOS and Windows, I believe that's safe (we dealt with a couple of thread-safety concerns back in bug 1375978), but it turns out that on Linux we still have a problem, because in order to set up the most appropriate FC_LANG for calling fontconfig, we use the LanguageAtomService, which may want to cache a lang->langGroup mapping in a hashtable.

Not sure yet what's the best way forward here. I think the most trivial "fix" might be to add a boolean 'false' as third parameter to the GetSampleLangForGroup call at:

https://searchfox.org/mozilla-central/rev/dc6d85680539cb7e5fe2843d38a30a0581bfefe1/gfx/thebes/gfxFcPlatformFontList.cpp#1762

Jean-Yves, could you try that and confirm whether it avoids the assertion? (If not, I may have misunderstood the relevant control flow that's happening here.) Thanks!
Flags: needinfo?(jfkthame) → needinfo?(jyavenard)
I should add that the defaut Font entry in the preference is always empty on this linux box. So the moving off the main thread wasn't really successful. Surprised that it wasn't picked up earlier.

Otherwise, adding False as 3rd parameter to https://searchfox.org/mozilla-central/rev/dc6d85680539cb7e5fe2843d38a30a0581bfefe1/gfx/thebes/gfxFcPlatformFontList.cpp#1762
does fix the issue, *and* the default font entry is now populated as expected.
Flags: needinfo?(jyavenard)
Calling this sec-low because it doesn't sound web triggerable (about:preferences is the only caller we found)
Is the main thread blocked while the font enumeration happens?  If not, then it seems like we should be acquiring the lock mentioned in the assertion, otherwise style worker threads could be touching the same objects.
This is a preliminary patch that just moves the relevant methods from gfxPlatformFontList to its linux/fontconfig-based subclass, as that is the only user of these methods so there's no reason for them to live in the cross-platform base class.
Attachment #8980685 - Flags: review?(emilio)
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
And this actually fixes the problem, by avoiding the unsafe cache usage when called from a non-main, non-servo-traversal thread.
Attachment #8980686 - Flags: review?(emilio)
(In reply to Cameron McCormack (:heycam) from comment #13)
> Is the main thread blocked while the font enumeration happens?  If not, then
> it seems like we should be acquiring the lock mentioned in the assertion,
> otherwise style worker threads could be touching the same objects.

No, AFAIK the main thread isn't blocked; so the patch above avoids the problem by simply not using the langGroup cache if we're on another thread. As this is only used to populate the Font menu in about:preferences (and only on Linux), I didn't think the slight performance penalty of bypassing the cache here would matter.
Comment on attachment 8980685 [details] [diff] [review]
patch 1 - Move methods only used by fontconfig backend from gfxPlatformFontList to the fontconfig subclass. (No functional change.)

Review of attachment 8980685 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thank you :)
Attachment #8980685 - Flags: review?(emilio) → review+
Comment on attachment 8980686 [details] [diff] [review]
patch 2 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use

Review of attachment 8980686 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/style/ServoUtils.h
@@ +33,5 @@
>    MOZ_ASSERT(sInServoTraversal || NS_IsMainThread());
>    return sInServoTraversal;
>  }
> +
> +inline bool IsInServoTraversalWithoutMainThreadAsserion()

Oops, typo.... obviously that should be ...Assertion() with a 't' in it!
Comment on attachment 8980686 [details] [diff] [review]
patch 2 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use

Review of attachment 8980686 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable, thanks for fixing this :)

I'm still somewhat concerned about all the complexity of the font-related code that we call OMT, and wish we could simplify it somehow... but that's not the point of this bug or patch in any case.

::: layout/style/ServoUtils.h
@@ +38,5 @@
> +{
> +  // This is for cases when we _only_ want to query whether we're in Servo
> +  // traversal, without asserting in the case of other non-main-thread callers.
> +  // Use this only if the calling code is safe to use off the main thread.
> +  return sInServoTraversal;

This looks somewhat easy to abuse, but yeah, can't think of anything better. Looks fine :)
Attachment #8980686 - Flags: review?(emilio) → review+
https://hg.mozilla.org/mozilla-central/rev/695a21084f8f
https://hg.mozilla.org/mozilla-central/rev/d48abd83fc7c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Hmm, actually I was wrong when reviewing this. Given this thread can run concurrently during the servo traversal, this can take the racy path incorrectly.

I'll revert the second part of this bug and reopen.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
To clarify, the second part, which I reverted in:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/ea82a59465a9

has the issue that a style thread may be running (and thus using the cache with the lock held) and this other font enumerator thread may end up in this block of code and IsInServoTraversalWithoutMainThreadAssertion() will return true as well (since it's a static variable).

So IsInServoTraversalWithoutMainThreadAssertion is not really useful, and we need to think of something else, either storing something in TLS to properly identify a Servo thread or font enumerator thread, or just expose the lock somewhere, or thread down the information of whether to use the cache somehow as an argument.
Comment on attachment 8980686 [details] [diff] [review]
patch 2 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use

Yeah, that's a problem.... what we really wanted to know there was something like IsServoTraversalThread, not just to check IsInServoTraversal (globally). But I guess that doesn't currently exist.

Probably the simplest solution is for the GetSystemFontList caller of GetSampleLangForGroup to pass in a flag saying that we should avoid using the langGroup cache.
Attachment #8980686 - Attachment is obsolete: true
This is just cleanup, removing the aCheckEnvironment parameter here because no-one changes it from its default.
Attachment #8981022 - Flags: review?(emilio)
And this should actually fix the bug (for real, this time).
Attachment #8981023 - Flags: review?(emilio)
Attachment #8981022 - Flags: review?(emilio) → review+
Comment on attachment 8981023 [details] [diff] [review]
patch 3 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use when called from GetSystemFontList

Review of attachment 8981023 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the lag, didn't want to review this by night :)

Looks good, thanks a lot for fixing.
Attachment #8981023 - Flags: review?(emilio) → review+
(In reply to Emilio Cobos Álvarez [:emilio] from comment #26)
> Sorry for the lag, didn't want to review this by night :)

No problem, I (probably) wouldn't have landed it while I was sleeping anyhow. ;)

Thanks for catching the issues here and making sure we fix it properly.
confirmed as fixed.
Status: RESOLVED → VERIFIED
Comment on attachment 8981023 [details] [diff] [review]
patch 3 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use when called from GetSystemFontList

Although this is sec-low (because it only applies to opening about:preferences, it's not a web-exposed vulnerability), I think we should take it on beta-61 and esr-60 so that Linux users aren't left with unsafe behavior (e.g. possible heap corruption) when opening Preferences.

Request is for the three patches as a set. Patches 1 and 2 here are purely cleanup to make the actual fix clearer; they involve no actual change to behavior. (Alternatively, we could backport only patch 3, but without the preliminary cleanup it would be easier to make a mistake in it.)

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
Minimal-risk fix that avoids exposing Linux users to a hashtable race condition that could result in....well, it's hard to say exactly what might go wrong as a result. Crashing would probably be the best-case outcome; subtle corruption could be worse.

User impact if declined: thread-unsafe hashtable usage when opening about:preferences on Linux, could result in crash or other corruption

Fix Landed on Version: 62

Risk to taking this patch (and alternatives if risky): minimal, this just ensures we skip use of the hashtable from the font-enumeration thread

String or UUID changes made by this patch: none

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1375978

[User impact if declined]: thread-safety violation when opening Preferences on Linux; could result in crash or other corruption

[Is this code covered by automated tests?]: no

[Has the fix been verified in Nightly?]: yes

[Needs manual test from QE? If yes, steps to reproduce]: yes - on Linux, with the LANGUAGE env variable set, open about:preferences using a DEBUG build; assertion should NOT be triggered

[List of other uplifts needed for the feature/fix]: none

[Is the change risky?]: no

[Why is the change risky/not risky?]: minimal behavior change, just avoids use of an unsafe cache from the about:preferences font-enumeration thread

[String changes made/needed]: none
Attachment #8981023 - Flags: approval-mozilla-esr60?
Attachment #8981023 - Flags: approval-mozilla-beta?
Comment on attachment 8981023 [details] [diff] [review]
patch 3 - Make gfxFcPlatformFontList::TryLangForGroup safe for off-main-thread use when called from GetSystemFontList

Avoids possibly-exploitable crashes when Linux users open the preferences page. Approved for 61.0b10 and ESR 60.1.
Attachment #8981023 - Flags: approval-mozilla-esr60?
Attachment #8981023 - Flags: approval-mozilla-esr60+
Attachment #8981023 - Flags: approval-mozilla-beta?
Attachment #8981023 - Flags: approval-mozilla-beta+
Whiteboard: [adv-main61+][adv-esr60.1+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.