Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2876493002:
Close NaCl IPC channel when the NaClDesc is released (Closed)
DescriptionClose NaCl IPC channel when the NaClDesc is released
NaClIPCAdapter has a ref count and a NaClDesc retains the reference to
it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a
IPC::Listener without retaining a reference.
When NaClDesc releases NaClIPCAdapter and it's being destroyed, it
schedules a task to destroy the channel. However, there's a gap between
the NaClIPCAdapter destruction and the channel destruction, and a chance
for the channel to call OnChannelError on NaClIPCAdapter, that causes
an UAF.
After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter.
BUG=719942
TBR=dschuff@chromium.org
Review-Url: https://codereview.chromium.org/2876493002
Cr-Commit-Position: refs/heads/master@{#476198}
Committed: https://chromium.googlesource.com/chromium/src/+/3d31416f453ded47bde9a2e2598be74a4ee943c3
Patch Set 1 #
Messages
Total messages: 25 (15 generated)
The CQ bit was checked by tzik@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Close NaCl IPC channel when the NaClDesc is released ========== to ========== Close NaCl IPC channel when the NaClDesc is released 123456789012345678901234567890123456789012345678901234567890123456789012 NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 ==========
tzik@chromium.org changed reviewers: + dschuff@chromium.org
Description was changed from ========== Close NaCl IPC channel when the NaClDesc is released 123456789012345678901234567890123456789012345678901234567890123456789012 NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 ========== to ========== Close NaCl IPC channel when the NaClDesc is released NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 ==========
PTAL
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
dschuff@chromium.org changed reviewers: + bbudge@chromium.org
looks OK to me, +bbudge
bbudge: ping. Could you review this?
lgtm
On 2017/05/25 15:25:55, bbudge wrote: > lgtm Thanks! dschuff: So, could you PTAL as an //components/nacl owner?
dschuff@google.com changed reviewers: + dschuff@google.com
The CQ bit was checked by dschuff@google.com
lgtm
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== Close NaCl IPC channel when the NaClDesc is released NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 ========== to ========== Close NaCl IPC channel when the NaClDesc is released NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 TBR=dschuff@chromium.org ==========
The CQ bit was checked by tzik@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1496288274837280, "parent_rev":
"cae7e7b126db785cf7fb1ae7c6ebc558f1c2b5eb", "commit_rev":
"3d31416f453ded47bde9a2e2598be74a4ee943c3"}
Message was sent while issue was closed.
Description was changed from ========== Close NaCl IPC channel when the NaClDesc is released NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 TBR=dschuff@chromium.org ========== to ========== Close NaCl IPC channel when the NaClDesc is released NaClIPCAdapter has a ref count and a NaClDesc retains the reference to it, and an IPC::Channel has a raw pointer to the NaClIPCAdapter as a IPC::Listener without retaining a reference. When NaClDesc releases NaClIPCAdapter and it's being destroyed, it schedules a task to destroy the channel. However, there's a gap between the NaClIPCAdapter destruction and the channel destruction, and a chance for the channel to call OnChannelError on NaClIPCAdapter, that causes an UAF. After this CL, NaClDesc closes the channel before it releases NaClIPCAdapter. BUG=719942 TBR=dschuff@chromium.org Review-Url: https://codereview.chromium.org/2876493002 Cr-Commit-Position: refs/heads/master@{#476198} Committed: https://chromium.googlesource.com/chromium/src/+/3d31416f453ded47bde9a2e2598b... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/3d31416f453ded47bde9a2e2598b... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
