Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick fefd6198da31 from chromium #35883

Merged
merged 6 commits into from Oct 12, 2022

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Oct 3, 2022

[M106] Fix races in FrameQueueUnderlyingSource related to CrossThreadPersistent

The repro in bug 1323488 unfortunately does not reliably reproduce
the races when run as a web test. I was also not able to repro the
races in a unit test.

There are actually three fixes in this CL; it was easiest to fix them
all together so that I can run the repro locally for an extended period
without it being stopped by a crash.

The underlying cause for all three races is that CrossThreadPersistent
can refer to an object whose heap has already been destroyed. When
accessed on the thread corresponding to that heap, there is no race,
but when accessed from a different thread, there is a period of time
after the heap is destroyed before the CrossThreadPersistent is
cleared.

  1. The FrameQueueUnderlyingSource::transferred_source_ member's pointer
    is accessed in FrameQueueUnderlyingSource::Close. This CL adds a
    callback to clear the pointer in
    TransferredFrameQueueUnderlyingSource::ContextDestroyed.

  2. The TransferredFrameQueueUnderlyingSource constructor takes a raw
    pointer to the original FrameQueueUnderlyingSource, which is
    associated with a different thread. GC won't be able to update this
    raw pointer since it's on the wrong stack. This CL changes the raw
    pointer to a CrossThreadPersistent which is visible to GC.

  3. Same as 2, but for the callstack ConnectHostCallback,
    MediaStream(Audio|Video)TrackUnderlyingSource::OnSourceTransferStarted
    and FrameQueueUnderlyingSource::TransferSource.

(cherry picked from commit 63ce9c40e1a67395278dfc70ecfb545a818747bb)

Bug: 1323488
Change-Id: Id63484eebefd2e003959b25bd752ac8263caab4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3865452
Commit-Queue: Ben Wagner benjaminwagner@google.com
Reviewed-by: Thomas Guilbert tguilbert@chromium.org
Auto-Submit: Ben Wagner benjaminwagner@google.com
Cr-Original-Commit-Position: refs/heads/main@{#1041434}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3908010
Commit-Queue: Srinivas Sista srinivassista@chromium.org
Reviewed-by: Srinivas Sista srinivassista@chromium.org
Cr-Commit-Position: refs/branch-heads/5249@{#521}
Cr-Branched-From: 4f7bea5de862aaa52e6bde5920755a9ef9db120b-refs/heads/main@{#1036826}

Ref electron/security#222

Notes: Security: backported fix for CVE-2022-3307.

@nornagon nornagon requested review from a team as code owners October 3, 2022 22:37
@nornagon nornagon added 20-x-y backport-check-skip Skip trop's backport validity checking security 🔒 semver/patch backwards-compatible bug fixes labels Oct 3, 2022
@electron-cation electron-cation bot added new-pr 🌱 PR opened in the last 24 hours and removed new-pr 🌱 PR opened in the last 24 hours labels Oct 3, 2022
@jkleinsc jkleinsc merged commit 9e064fe into 20-x-y Oct 12, 2022
@jkleinsc jkleinsc deleted the cherry-pick/20-x-y/chromium/fefd6198da31 branch October 12, 2022 13:53
@release-clerk
Copy link

release-clerk bot commented Oct 12, 2022

Release Notes Persisted

Security: backported fix for CVE-2022-3307.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
20-x-y backport-check-skip Skip trop's backport validity checking security 🔒 semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants