Closed Bug 1633467 Opened 5 years ago Closed 4 years ago

Crash in [@ Allocator<T>::free | replace_free | FcFreeTypeQueryFaceInternal]

Categories

(Core :: Graphics: Text, defect, P3)

Product:
Core
Component:
Graphics: Text
Platform:
Unspecified
Linux
Type:
defect

Tracking

()

Status:
RESOLVED WORKSFORME

People

(Reporter: gsvelto, Unassigned)

References

Details

(Keywords: crash)

Crash Data

This bug is for crash report bp-adc96001-6cdb-48aa-9723-c32aa0200425.

Top 10 frames of crashing thread:

0 firefox-bin Allocator<MozJemallocBase>::free memory/build/malloc_decls.h:54
1 firefox-bin replace_free memory/replace/phc/PHC.cpp:1312
2 libfontconfig.so.1 FcFreeTypeQueryFaceInternal ./src/fcfreetype.c:2125
3 libxul.so gfxFontconfigFontEntry::gfxFontconfigFontEntry gfx/thebes/gfxFcPlatformFontList.cpp:316
4 libxul.so gfxFcPlatformFontList::MakePlatformFont gfx/thebes/gfxFcPlatformFontList.cpp:1900
5 libxul.so gfxUserFontEntry::LoadPlatformFont gfx/thebes/gfxUserFontSet.cpp:795
6 libxul.so gfxUserFontEntry::LoadPlatformFontSync gfx/thebes/gfxUserFontSet.cpp:704
7 libxul.so gfxUserFontEntry::DoLoadNextSrc gfx/thebes/gfxUserFontSet.cpp:606
8 libxul.so void gfxFontGroup::InitScriptRun<char16_t> gfx/thebes/gfxUserFontSet.cpp:469
9 libxul.so gfxFontGroup::MakeTextRun gfx/thebes/gfxTextRun.cpp:2469

This looks like a double-free. It's probably a regression since it's only on nightly and it started with buildid 20200412214314.

Flags: needinfo?(jfkthame)

I looked at a number of the crash reports, and it appeared they were probably all coming from the same installation (I didn't check every one, though, so there might be exceptions). This makes me wonder whether it's specific to a certain fontconfig (and/or freetype) version that user has installed.

I'm not aware of any Gecko changes that seem to correlate with the timing of this; the one vaguely-plausible thing that comes to mind is bug 1547063, but that landed back in the Firefox 71 cycle, so it's been around for a while. Maybe Lee has some other ideas?

Flags: needinfo?(jfkthame) → needinfo?(lsalzman)

There's at least six installations sending crashes though one of them is responsible for ~90% of the reports. One interesting bit is that the crashes are all coming from Debian bullseye/sid installations so it might be related to some of the packages available for that version of Debian.

Here's a hint of what might have caused this: the libfontconfig1 package was updated on Debian on the 21st of April and the first crash we have on record was sent on the 22nd. Here's the changelog:

2020-04-21 - Emilio Pozuelo Monfort <pochu@debian.org>
fontconfig (2.13.1-4) unstable; urgency=medium
* Backport upstream patches to fix a few memory leaks. Closes: #956157.
* The last upload had to go through NEW, so this source-only upload will
allow fontconfig to be a candidate for testing migration.
Closes: #958106.

We might need some help from their side to fix this.

Huh, that certainly looks suspicious.

Specifically, I think that https://gitlab.freedesktop.org/fontconfig/fontconfig/-/commit/61573ad5f7c4dd0860d613d99d0086433240eb75, which was one of the backported changesets, is buggy. Looking at the FcFreeTypeQueryFaceInternal function, it may allocate a name_mapping with malloc (if one was not passed in as *nm_share); and it may then free it after the big for (p = 0; p < NUM_PLATFORM_ORDER; p++) loop.

But after this (so name_mapping may have been freed), there are lots of error cases that jump to the bail1 label, and the cleanup code there will again try to free(name_mapping) if no nm_share was passed. So AFAICS that would indeed be a double-free.

I'll try to report this upstream to fontconfig, but any double-checking of my understanding would also be welcome here.

Priority: -- → P3
Severity: -- → normal

(In reply to Jonathan Kew (:jfkthame) from comment #4)

Huh, that certainly looks suspicious.

Specifically, I think that https://gitlab.freedesktop.org/fontconfig/fontconfig/-/commit/61573ad5f7c4dd0860d613d99d0086433240eb75, which was one of the backported changesets, is buggy. Looking at the FcFreeTypeQueryFaceInternal function, it may allocate a name_mapping with malloc (if one was not passed in as *nm_share); and it may then free it after the big for (p = 0; p < NUM_PLATFORM_ORDER; p++) loop.

But after this (so name_mapping may have been freed), there are lots of error cases that jump to the bail1 label, and the cleanup code there will again try to free(name_mapping) if no nm_share was passed. So AFAICS that would indeed be a double-free.

I'll try to report this upstream to fontconfig, but any double-checking of my understanding would also be welcome here.

That assessment seems accurate, as far as I can see looking through the patch and the FC code.

Flags: needinfo?(lsalzman)

I can reproduce this consistently by loading https://www.gov.uk/bank-holidays.

Yeah, the gov.uk site is using a webfont that trips over a fontconfig bug, see above. The workaround would be to either patch or downgrade your libfontconfig.

From bug 1629231, an alternative workaround (less desirable):

gfx.downloadable_fonts.enabled false

Note that the FreeBSD folks have just fixed their version of this, see bug 1629231 and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=245915. I don't think there's anything we can really do here in Gecko, it just needs to get fixed similarly in Debian.

I've poked the Debian fontconfig maintainer just now.

Awesome, thanks. The fontconfig patch was just merged upstream, so if they update to latest master they should be OK.

The severity field is not set for this bug.
:lsalzman, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(lsalzman)

I guess we can call this fixed.

Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(lsalzman)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.