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

Issue 830907 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
please use my google.com address
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 821030



Sign in to add a comment

third_party/WebKit/LayoutTests/mojo/message-pipe.html operates on invalid mojo handle

Project Member Reported by xunji...@chromium.org, Apr 9 2018

Issue description

Steps to repro:
(1) Apply this patch https://chromium-review.googlesource.com/c/chromium/src/+/957918 which adds a DCHECK if caller tries to dereference an invalid mojo handle.

(2) Run webkit_layout_tests, and observe that mojo/message-pipe.html will fail the added DCHECK

13:34:27.485 2058 worker/1 mojo/message-pipe.html crashed, (stderr lines):
13:34:27.485 2058   
13:34:27.485 2058   DevTools listening on ws://127.0.0.1:34152/devtools/browser/9210dae6-d11d-4dc6-8e36-3262673b147f
13:34:27.485 2058   [1:1:0409/133427.263467:FATAL:handle.h(98)] Check failed: handle_.is_valid(). 
13:34:27.485 2058   #0 0x0000035151ac base::debug::StackTrace::StackTrace()
13:34:27.485 2058   #1 0x0000035356bb logging::LogMessage::~LogMessage()
13:34:27.485 2058   #2 0x000005c590c9 blink::MojoHandle::readMessage()
13:34:27.485 2058   #3 0x000004d4e88a blink::V8MojoHandle::readMessageMethodCallback()
13:34:27.485 2058   #4 0x0000020a1c13 v8::internal::FunctionCallbackArguments::Call()
13:34:27.485 2058   #5 0x0000020a003d v8::internal::(anonymous namespace)::HandleApiCallHelper<>()
13:34:27.485 2058   #6 0x00000209dfb8 v8::internal::Builtin_Impl_HandleApiCall()
13:34:27.485 2058   #7 0x00000209d9fd v8::internal::Builtin_HandleApiCall()
13:34:27.485 2058   #8 0x2cf2ec604344 <unknown>
13:34:27.485 2058   
13:34:27.487 30054 [1/2] mojo/message-pipe.html failed unexpectedly (renderer crashed)
 
Cc: roc...@chromium.org
+rockot@: could you help to triage this issue? 
I am not familiar with mojo's JS APIs.
Cc: -roc...@chromium.org
Owner: roc...@chromium.org
Status: Started (was: Untriaged)
Sure, thanks for bringing this to my attention. Fix is pretty trivial, I'll upload a CL soon.
Try with https://chromium-review.googlesource.com/c/chromium/src/+/1003287 applied too. Should be fine after that.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

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

commit 4339359d5a7df991d3e6bdd8707050152df9ad6a
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Apr 10 04:21:25 2018

Mojo Blink: Don't dereference potentially invalid handles

mojo::ScopedHandleBase::operator-> is imminently changing to DCHECK that
it's only called on valid handles. Blink's MojoHandle wrapper is
explicitly intended to be able to represent invalid handle values and
forward them to underlying system API calls.

This changes Blink's MojoHandle to use get() instead of operator-> to
get the underlying value of its wrapped handle, as get() makes no
assertions or guarantees about the validity of the handle value.

Bug:  830907 
Change-Id: If0b5b9e929e78aafef868330a537ec20c3ac1deb
Reviewed-on: https://chromium-review.googlesource.com/1003287
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549418}
[modify] https://crrev.com/4339359d5a7df991d3e6bdd8707050152df9ad6a/third_party/blink/renderer/core/mojo/mojo_handle.cc

Comment 5 by roc...@chromium.org, Apr 10 2018

Status: Fixed (was: Started)

Sign in to add a comment