1694670 - Linux startup crash in [@ qcms_data_create_rgb_with_gamma]

Status: RESOLVED FIXED

(Core :: Graphics: Color Management, defect)

Description

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

90 crashes with various Linux distributions in the last 6 weeks, some have beta 0 as version (distros testing?).

Crash report: https://crash-stats.mozilla.org/report/index/2a7dee73-3a4d-490a-96fd-4af7f0210224

MOZ_CRASH Reason: OOB

Top 10 frames of crashing thread:

0 libxul.so RustMozCrash mozglue/static/rust/wrappers.cpp:17
1 libxul.so mozglue_static::panic_hook mozglue/static/rust/lib.rs:89
2 libxul.so core::ops::function::Fn::call /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/core/src/ops/function.rs:70
3 libxul.so std::panicking::rust_panic_with_hook library/std/src/panicking.rs:595
4 libxul.so std::panicking::begin_panic::{{closure}} /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:520
5 libxul.so std::sys_common::backtrace::__rust_end_short_backtrace /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:141
6 libxul.so std::panicking::begin_panic /builds/worker/fetches/rustc/lib/rustlib/src/rust/library/std/src/panicking.rs:519
7 libxul.so qcms_data_create_rgb_with_gamma gfx/qcms/src/c_bindings.rs:287
8 libxul.so gfxPlatformGtk::GetPlatformCMSOutputProfileData gfx/thebes/gfxPlatformGtk.cpp:483
9 libxul.so gfxPlatform::Init gfx/thebes/gfxPlatform.cpp:1005

Comment 1

[Jeff Muizelaar [:jrmuizel]]

It doesn't seem like this should be a security bug as it is just a rust panic on startup.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Bug 1694670 - Fix qcms_data_create_rgb_with_gamma.

This fixes a number of problems:

1. The check around get_rgb_colorants was inverted. This caused us to
   only continue if the colorants were wrong.

2. get_rgb_colorants can just return the Matrix instead of taking
   a reference to it.

3. The OOBs checks in write_u32 and write_u16 had their conditions
   inverted.

4. No tests

Comment 3

[Jeff Muizelaar [:jrmuizel]]

We should just fix the reversed OOB checks here and do the other stuff elsewhere.

Comment 4

[Daniel Veditz [:dveditz]]

So these particular crashes may not be scary, but qcms_data_create_rgb_with_gamma is a very large unsafe function so are we sure there aren't potentially vulnerable crashes if we've reversed the conditions?

Comment 5

[Jeff Muizelaar [:jrmuizel]]

qcms_data_create_rgb_with_gamma is only called on system local data (i.e information from the user's window server). There shouldn't be any way to exploit it. Further, the out of bounds checks were only added recently, previously there was no check at all.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Attachment: Bug 1694670. Fix the OOB check in write_u32/u16.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 9205268 [details]
Bug 1694670. Fix the OOB check in write_u32/u16.

Beta/Release Uplift Approval Request

• User impact if declined: This fixes a startup crash that happens when users have an invalid color profile on Linux
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This code path is very rare as evidenced by the low crash rate. This patch restores the behaviour to what it was prior to being regressed by bug 1684095
• String changes made/needed:

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/95fc70920b71
Fix the OOB check in write_u32/u16. r=aosmond

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9205268 [details]
Bug 1694670. Fix the OOB check in write_u32/u16.

Approved for 87.0b3 so we can get feedback on this ASAP.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e24e2d039a0317d0d66bdb041df65792ae69f555

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/95fc70920b71

Comment 13

[Gabriele Svelto [:gsvelto]]

Ugh, this bug is affecting Linux users real badly on 86. Do we have a way to reach out to distro maintainers to try and help them roll out a local fix for their 86 builds?

Comment 14

[Julien Cristau [:jcristau]]

(In reply to Gabriele Svelto [:gsvelto] from comment #13)

Ugh, this bug is affecting Linux users real badly on 86. Do we have a way to reach out to distro maintainers to try and help them roll out a local fix for their 86 builds?

adding a few of them here.

Comment 15

[Martin Stránský [:stransky] (ni? me)]

I shipped Fedora Firefox updates yesterday, Thanks.

Comment 16

[Olivier Tilloy]

This was also reported in Ubuntu (https://launchpad.net/bugs/1917191), by only 2 different users so far since the update to 86.0 was pushed to all supported Ubuntu releases. According to comment 7, this affects only users with an invalid color profile on Linux, which presumably/hopefully isn't such a large number of users.

I'm okay with distro-patching (the patch is trivial enough), but then why isn't the change being uplifted to the 86 branch?

Comment 17

[Robert Mader [:rmader]]

Jan, this might be important for you as well (archlinux).

Comment 18

[Jan Alexander Steffens [:heftig]]

I've already backported this fix; thanks.

Comment 20

[Pascal Chevrel:pascalc]

Comment on attachment 9205268 [details]
Bug 1694670. Fix the OOB check in write_u32/u16.

Approved for 86.0.1, thanks.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/fdc6e37202db

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Added to the 86.0.1 relnotes:

Fixed a frequent Linux crash on browser launch