Closed Bug 1850615 Opened 3 years ago Closed 1 year ago

Crash in [@ MediaLibvaCaps::CheckAttribList] (RDD crash, Linux, KDE neon)

Categories

(Core :: Audio/Video: Playback, defect)

Product:
Core
Component:
Audio/Video: Playback
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
138 Branch
Tracking Flags:
Tracking Status
firefox-esr115 --- wontfix
firefox-esr128 137+ fixed
firefox119 --- wontfix
firefox136 --- wontfix
firefox137 + fixed
firefox138 + fixed

People

(Reporter: Sylvestre, Assigned: karlt)

References

Details

(4 keywords, Whiteboard: [adv-main137+r][adv-esr128.9+r] )

Crash Data

Attachments

(1 file, 6 external files, 2 obsolete files)

about:support
70.92 KB, text/plain
Details

Crash report: https://crash-stats.mozilla.org/report/index/22ece291-fcd1-458d-a19b-5e4c60230820

Reason: SIGSEGV / SI_KERNEL

Top 10 frames of crashing thread:

0  iHD_drv_video.so  MediaLibvaCaps::CheckAttribList  obj-x86_64-linux-gnu/media_driver/media_driver/linux/common/ddi/media_libva_caps.cpp:475
1  iHD_drv_video.so  MediaLibvaCaps::CreateConfig  obj-x86_64-linux-gnu/media_driver/media_driver/linux/common/ddi/media_libva_caps.cpp:2313
2  libva.so.2  vaCreateConfig  va/va/va.c:934
3  libmozavcodec.so  vaCreateConfig  media/mozva/mozva.c:278
3  libmozavcodec.so  vaapi_decode_make_config  media/ffvpx/libavcodec/vaapi_decode.c:547
4  libmozavcodec.so  ff_vaapi_decode_init  media/ffvpx/libavcodec/vaapi_decode.c:680
5  libmozavcodec.so  hwaccel_init  media/ffvpx/libavcodec/decode.c:1235
5  libmozavcodec.so  ff_get_format  media/ffvpx/libavcodec/decode.c:1373
6  libmozavcodec.so  get_pixel_format  media/ffvpx/libavcodec/av1dec.c:611
6  libmozavcodec.so  av1_receive_frame_internal  media/ffvpx/libavcodec/av1dec.c:1228

I have often OOM issue on this system

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

For more information, please visit BugBot documentation.

Flags: needinfo?(jmathies)

Sylvestre, can you share about:support of the machine on which you see this?

Flags: needinfo?(jmathies) → needinfo?(sledru)
Attached file about:support —
Flags: needinfo?(sledru)

Martin, is this something you know of? Crash when attempting to decode AV1 with an intel HW decoder.

Flags: needinfo?(stransky)

These crashes, including the one in comment 0, are happening on the jemalloc poison value, 0xe5e5e5e5e5e5e5f5, indicating a use-after-free.

Group: media-core-security
Keywords: csectype-uaf, sec-high

(In reply to Paul Adenot (:padenot) from comment #4)

Martin, is this something you know of? Crash when attempting to decode AV1 with an intel HW decoder.

No idea. Sylvestre, can you reproduce it? For instance do you have an AV1 clip which can trigger it?

Flags: needinfo?(stransky) → needinfo?(sledru)

I can't reproduce it, sorry :(

Flags: needinfo?(sledru)

Two reports, RDD crash in an intel driver on Linux. Only one user who can't reproduce. I guess we could block this driver but since we're not seeing this 'in the wild', might be worth simply monitoring.

Summary: Crash in [@ MediaLibvaCaps::CheckAttribList] → Crash in [@ MediaLibvaCaps::CheckAttribList] (RDD crash, Linux, KDE neon)

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

For more information, please visit BugBot documentation.

Flags: needinfo?(jmathies)
Severity: -- → S2
Flags: needinfo?(jmathies)
Severity: S2 → S3

This was happening rather frequently late in August, but it hasn't occurred since September 11, so let's just close this, given that it sounds like there are no ideas for paths forward here.

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME

This has been present at a low level the whole 6 month crash window I can see, but really took off again a month ago in 136 nightlies, all with Intel graphics. No particular driver version: 24.0.9.0 is the most common (about a third of the crashes), but there are some 22.x, 23.x, and newer 24.x versions represented.

I don't see any 137 nightly crashes yet so maybe this is an alternate presentation of something we fixed. The stacks still look like comment 0
bp-b263c8e3-9e7c-4d05-8120-ecd9c0250204

Apart from the driver-specific top of the stack, they also look very much like the nVidia crash bug 1946407

Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

The severity field for this bug is set to S3. However, the bug is flagged with the sec-high keyword.
:jimm, could you consider increasing the severity of this security bug?

For more information, please visit BugBot documentation.

Flags: needinfo?(jmathies)
Blocks: media-triage
Flags: needinfo?(jmathies)

Hey Karl, wondering if you might have some ideas?

Flags: needinfo?(karlt)

This is about the lifetime of a VADriverVTable. Gecko or ffmpeg might plausibly be influencing the lifetime through vaTerminate(VADisplay).

Martin, do you know whether the default false value of "media.ffmpeg.vaapi.enabled" means that only users that have enabled this feature under development would be using the code involved here?
Or might ffmpeg/ffvpx bypass this pref?

Flags: needinfo?(karlt) → needinfo?(stransky)

(In reply to Karl Tomlinson (:karlt) from comment #15)

This is about the lifetime of a VADriverVTable. Gecko or ffmpeg might plausibly be influencing the lifetime through vaTerminate(VADisplay).

Martin, do you know whether the default false value of "media.ffmpeg.vaapi.enabled" means that only users that have enabled this feature under development would be using the code involved here?
Or might ffmpeg/ffvpx bypass this pref?

media.ffmpeg.vaapi.enabled is here from historical reasons, we may remove it.
VA-API decoding on Linux follows 'media.hardware-video-decoding.enabled' pref and it's enabled by default on recent MESA and AMD/Intel drivers.

Flags: needinfo?(stransky)

DdiMediaProtected::FreeInstances() deletes all DdiMediaProtected pointees of the singleton map DdiMediaProtected::_impl. The pointers are reset to null, but this is not atomic. DdiMediaProtected is not reference counted.

DdiMediaProtected::GetInstance() will return a DdiMediaProtected* from _impl if non-null or create a new DdiMediaProtected. The key is apparently always DDI_PROTECTED_CONTENT.

DdiMediaProtected::FreeInstances() is called from DdiMedia_Terminate(), an implementation of vaTerminate(), with media_libva.cpp's singleton GlobalMutex is held. This prevents concurrency of FreeInstances() calls, but the mutex is not held during GetInstance() calls. GlobalMutex is also held during DdiMedia_InitMediaContext(), but this is not when GetInstance() calls are made.

When GetInstance() and FreeInstances() run on different threads GetInstance() can fetch a DdiMediaProtected* before it is deleted.

The driver seems to think that only one VADisplay will be in use concurrently.

vaTerminate() is triggered on destruction of Gecko's implementation of a VAAPI AVHWDeviceContext.

When two of these exist concurrently, vaTerminate() may be called for one while the other is in use for calls into libva.

Perhaps a single VADisplay can be shared between AVHWDeviceContext objects so that vaTerminate() is not called until libva is unused.

This is hoping that nothing else will use libva from the same process.

I don't know when the Intel driver became used by default.

Anything that changed the frequency or timing of decoder destruction may have changed the likelihood of the race running badly.

Keywords: csectype-race

Bug 1752494 enabled this for Nightly and bug 1777430 for release.

Regressions: egl-linux-vaapi-release-intel, egl-linux-vaapi-nightly
Assignee: nobody → karlt
No longer blocks: media-triage
Status: REOPENED → ASSIGNED

Comment on attachment 9468838 [details]
Bug 1850615 Share the same libva handle across all FFmpegVideoDecoders r?stransky

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: The UaF involves a race where there is a short window of opportunity between the delete()ion of the vtable object and the nulling of its pointer on the same thread.
    The crash occurs in the Remote Data Decoder process which is sandboxed.
    Content could most likely influence the timing of the delete() and the concurrent reference.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
  • Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: Beta, Release, ESR115, ESR128, and yes
  • If not all supported branches, which bug introduced the flaw?: Bug 1777430
  • Do you have backports for the affected branches?: No
  • If not, how different, hard to create, and risky will they be?: There will be some merge conflicts to resolve due to the size of the patch, but the relevant code has not changed in behavior, so additional risk is not significant.
  • How likely is this patch to cause regressions; how much testing does it need?: Due to the interaction with third party libraries and the size of the patches, this would benefit from some Nightly exposure.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: No
Attachment #9468838 - Flags: sec-approval?
Attachment #9468839 - Flags: sec-approval?

Comment on attachment 9468839 [details]
Bug 1850615 Share the same VADisplay across all FFmpegVideoDecoders r?stransky

Approved to land and request uplift when ready

Attachment #9468839 - Flags: sec-approval? → sec-approval+

Comment on attachment 9468838 [details]
Bug 1850615 Share the same libva handle across all FFmpegVideoDecoders r?stransky

Approved to land and request uplift when ready

Attachment #9468838 - Flags: sec-approval? → sec-approval+
Pushed by ktomlinson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b34601c7751d Share the same libva handle across all FFmpegVideoDecoders r=stransky
Pushed by ktomlinson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9d547b90a407 Share the same VADisplay across all FFmpegVideoDecoders r=stransky

https://hg.mozilla.org/mozilla-central/rev/b34601c7751d
https://hg.mozilla.org/mozilla-central/rev/9d547b90a407

Group: media-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 3 years ago → 1 year ago
status-firefox138: affected → fixed
status-firefox-esr115: affected → wontfix
tracking-firefox137: --- → +
tracking-firefox138: --- → +
tracking-firefox-esr128: --- → 137+
Resolution: --- → FIXED
Target Milestone: --- → 138 Branch

The patch landed in nightly and beta is affected.
:karlt, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regressions caused by this fix.
  • If no, please set status-firefox137 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(karlt)

With the latest trunk I'm getting (on debug):

[20727] Assertion failure: int32_t(mRefCnt) > 0 (dup release), at /home/komat/src2/dom/media/platforms/ffmpeg/VALibWrapper.cpp:128

#6  0x00007f4dc111774b in MOZ_CrashSequence (aAddress=0x0, aLine=128) at /home/komat/src2/objdir-opt/dist/include/mozilla/Assertions.h:267
#7  mozilla::VADisplayHolder::MaybeDestroy (this=0x7f4db83e8740) at /home/komat/src2/dom/media/platforms/ffmpeg/VALibWrapper.cpp:128
#8  0x00007f4dc111184b in mozilla::VADisplayHolder::Release (this=0x7f4db83e8740) at /home/komat/src2/dom/media/platforms/ffmpeg/VALibWrapper.h:59
#9  0x00007f4db0e35f6d in hwdevice_ctx_free (opaque=<optimized out>, data=0x7f4db83e6740 "H\262\351\260M\177") at /home/komat/src2/media/ffvpx/libavutil/hwcontext.c:153
#10 0x00007f4db0e2a3f1 in buffer_replace (dst=<optimized out>, src=0x0) at /home/komat/src2/media/ffvpx/libavutil/buffer.c:133
#11 av_buffer_unref (buf=<optimized out>) at /home/komat/src2/media/ffvpx/libavutil/buffer.c:144
#12 0x00007f4db0889cb6 in ff_codec_close (avctx=0x7f4dcb971000) at /home/komat/src2/media/ffvpx/libavcodec/avcodec.c:484
#13 0x00007f4db0889ea9 in avcodec_close (avctx=0x7f4dcbdc9760 <_IO_stdfile_2_lock>) at /home/komat/src2/media/ffvpx/libavcodec/avcodec.c:503
#14 0x00007f4dc1100f12 in mozilla::FFmpegDataDecoder<46465650>::ProcessShutdown (this=this@entry=0x7f4db83f0a00) at /home/komat/src2/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:280
#15 0x00007f4dc110bdd1 in mozilla::FFmpegVideoDecoder<46465650>::ProcessShutdown (this=0x7f4db83f0a00) at /home/komat/src2/dom/media/platforms/ffmpeg/FFmpegVideoDecoder.cpp:1785
#16 0x00007f4dc111300b in mozilla::FFmpegDataDecoder<46465650>::Shutdown()::$_0::operator()() const (this=0x7f4d9886f070) at /home/komat/src2/dom/media/platforms/ffmpeg/FFmpegDataDecoder.cpp:153
#17 mozilla::detail::ProxyFunctionRunnable<mozilla::FFmpegDataDecoder<46465650>::Shutdown()::$_0, mozilla::MozPromise<bool, bool, false> >::Run (this=0x7f4d989a1ec0) at /home/komat/src2/objdir-opt/dist/include/mozilla/MozPromise.h:1821

Open video clip from disk and close Firefox window with alt+f4.

Regressions: 1952783
Attachment #9471288 - Flags: approval-mozilla-beta?
Attachment #9471289 - Flags: approval-mozilla-beta?

beta Uplift Approval Request

  • User impact if declined: sec-high
  • Code covered by automated testing: no
  • Fix verified in Nightly: no
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: n/a
  • Risk associated with taking this patch: moderate
  • Explanation of risk level: The size of the patches and the interaction with third party libraries means there is some risk affecting Linux-only video playback.
  • String changes made/needed: None
  • Is Android affected?: no
Attachment #9471291 - Flags: approval-mozilla-beta?
Attachment #9471292 - Flags: approval-mozilla-beta?
Attachment #9471292 - Attachment is obsolete: true
Attachment #9471292 - Flags: approval-mozilla-beta?
Attachment #9471291 - Attachment is obsolete: true
Attachment #9471291 - Flags: approval-mozilla-beta?
Attachment #9471302 - Flags: approval-mozilla-esr128?
Attachment #9471303 - Flags: approval-mozilla-esr128?

esr128 Uplift Approval Request

  • User impact if declined: sec-high
  • Code covered by automated testing: no
  • Fix verified in Nightly: no
  • Needs manual QE test: no
  • Steps to reproduce for manual QE testing: n/a
  • Risk associated with taking this patch: moderate
  • Explanation of risk level: The size of the patches and the interaction with third party libraries means there is some risk affecting Linux-only video playback.
  • String changes made/needed: none
  • Is Android affected?: no
Attachment #9471289 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9471288 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9471303 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
Attachment #9471302 - Flags: approval-mozilla-esr128? → approval-mozilla-esr128+
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Whiteboard: [adv-main137+r]
Whiteboard: [adv-main137+r] → [adv-main137+r][adv-esr128.9+r]
Regressions: 1958036
Flags: needinfo?(karlt)
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: