Closed Bug 1694670 Opened 9 months ago Closed 9 months ago

Linux startup crash in [@ qcms_data_create_rgb_with_gamma]

Categories

(Core :: GFX: Color Management, defect)

defect

Tracking

()

RESOLVED FIXED
88 Branch
Tracking Status
relnote-firefox --- 86+
firefox-esr78 --- unaffected
firefox86 + fixed
firefox87 + fixed
firefox88 + fixed

People

(Reporter: aryx, Assigned: jrmuizel)

References

(Regression)

Details

(4 keywords)

Crash Data

Attachments

(1 file, 1 obsolete file)

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
Flags: needinfo?(jmuizelaar)

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

Flags: needinfo?(jmuizelaar)

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

Assignee: nobody → jmuizelaar
Status: NEW → ASSIGNED
Regressed by: 1684095
No longer regressed by: 1684517
Attachment #9205116 - Attachment is obsolete: true

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

Attachment #9205116 - Attachment is obsolete: false

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?

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.

Group: gfx-core-security

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:
Attachment #9205268 - Flags: approval-mozilla-release?
Attachment #9205268 - Flags: approval-mozilla-beta?
Attachment #9205116 - Flags: approval-mozilla-release?
Attachment #9205116 - Flags: approval-mozilla-beta?
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/95fc70920b71
Fix the OOB check in write_u32/u16. r=aosmond
Attachment #9205116 - Attachment is obsolete: true
Attachment #9205116 - Flags: approval-mozilla-release?
Attachment #9205116 - Flags: approval-mozilla-beta?

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.

Attachment #9205268 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch
Duplicate of this bug: 1694891

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?

(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.

Flags: needinfo?(stransky)
Flags: needinfo?(ricotz)
Flags: needinfo?(olivier)
Flags: needinfo?(mh+mozilla)

I shipped Fedora Firefox updates yesterday, Thanks.

Flags: needinfo?(stransky)

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?

Flags: needinfo?(olivier)

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

Flags: needinfo?(jan.steffens)

I've already backported this fix; thanks.

Flags: needinfo?(jan.steffens)
See Also: → 1696411
See Also: 1696411
Duplicate of this bug: 1696411
Flags: needinfo?(mh+mozilla)

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

Approved for 86.0.1, thanks.

Attachment #9205268 - Flags: approval-mozilla-release? → approval-mozilla-release+

Added to the 86.0.1 relnotes:

Fixed a frequent Linux crash on browser launch

Flags: needinfo?(ricotz)
Duplicate of this bug: 1696339
You need to log in before you can comment on or make changes to this bug.