New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 752137 link

Starred by 1 user
Status: Fixed
Owner:
█ roc...@chromium.org
please use my google.com address
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Mojo: Missing RequestContext on Channel error

Project Member Reported by roc...@chromium.org, Aug 3 2017 
In the event that an internal Channel error is posted to the IPC thread task queue for async dispatch, we fail to construct a RequestContext on the thread. If the error processing leads to any watcher event notifications, they'll result in a nullptr deref and crash.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 3 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/00d59f5db8d573a372605669ce6958ebced42db1

commit 00d59f5db8d573a372605669ce6958ebced42db1
Author: Ken Rockot <rockot@chromium.org>
Date: Thu Aug 03 16:16:39 2017

Mojo: Ensure a RequestContext exists on async Channel error processing

If a channel error is triggered from off-thread and posted to the IPC
thread, we weren't ensuring that a RequestContext was in TLS. If the
error lead to event notifications this would cause a nullptr deref.

Fixes that.

BUG= 752137 
TBR=jcivelli@chromium.org

Change-Id: Iae167a712c9b7d8d853fe805c06afef36d2c4572
Reviewed-on: https://chromium-review.googlesource.com/600291
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491758}
[modify] https://crrev.com/00d59f5db8d573a372605669ce6958ebced42db1/mojo/edk/system/node_controller.cc

Comment 2 by roc...@chromium.org, Aug 3 2017

Labels: Merge-Request-61
Status: Fixed (was: Started)
Low-risk trivial fix for potential crasher.
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 4 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 4 2017

Labels: -merge-approved-61 merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6f81ee52c3cd74b6017e6b64eb30f0203581a8f5

commit 6f81ee52c3cd74b6017e6b64eb30f0203581a8f5
Author: Ken Rockot <rockot@chromium.org>
Date: Fri Aug 04 16:57:10 2017

Mojo: Ensure a RequestContext exists on async Channel error processing

If a channel error is triggered from off-thread and posted to the IPC
thread, we weren't ensuring that a RequestContext was in TLS. If the
error lead to event notifications this would cause a nullptr deref.

Fixes that.

BUG= 752137 
TBR=jcivelli@chromium.org, rockot@chromium.org

(cherry picked from commit 00d59f5db8d573a372605669ce6958ebced42db1)

Change-Id: Iae167a712c9b7d8d853fe805c06afef36d2c4572
Reviewed-on: https://chromium-review.googlesource.com/600291
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491758}
Reviewed-on: https://chromium-review.googlesource.com/602491
Cr-Commit-Position: refs/branch-heads/3163@{#314}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/6f81ee52c3cd74b6017e6b64eb30f0203581a8f5/mojo/edk/system/node_controller.cc
►

Sign in to add a comment