Commit b56e0af1 authored by Kevin McNee's avatar Kevin McNee Committed by Commit Bot

Add crash keys to investigate the GetGuestByInstanceIDSafely kill

Add crash keys to identify why the forbidden access is happening and
which WebView extension function(s) are involved in the kill.

Bug: 780728
Change-Id: I7b68103696ef3c5c46a5e00299aceef422e136bd
Reviewed-on: https://chromium-review.googlesource.com/c/1344874Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarEhsan Karamad <ekaramad@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610140}
parent 9e4c2b66
include_rules = [ include_rules = [
"+components/crash/core/common/crash_key.h",
"+components/zoom", "+components/zoom",
"+content/public/browser", "+content/public/browser",
"+content/public/common", "+content/public/common",
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "components/crash/core/common/crash_key.h"
#include "components/guest_view/browser/bad_message.h" #include "components/guest_view/browser/bad_message.h"
#include "components/guest_view/browser/guest_view_base.h" #include "components/guest_view/browser/guest_view_base.h"
#include "components/guest_view/browser/guest_view_manager_delegate.h" #include "components/guest_view/browser/guest_view_manager_delegate.h"
...@@ -462,17 +463,24 @@ bool GuestViewManager::GetFullPageGuestHelper( ...@@ -462,17 +463,24 @@ bool GuestViewManager::GetFullPageGuestHelper(
bool GuestViewManager::CanEmbedderAccessInstanceID( bool GuestViewManager::CanEmbedderAccessInstanceID(
int embedder_render_process_id, int embedder_render_process_id,
int guest_instance_id) { int guest_instance_id) {
// TODO(780728): Remove crash key once the cause of the kill is known.
static crash_reporter::CrashKeyString<32> bad_access_key("guest-bad-access");
// The embedder is trying to access a guest with a negative or zero // The embedder is trying to access a guest with a negative or zero
// instance ID. // instance ID.
if (guest_instance_id <= kInstanceIDNone) if (guest_instance_id <= kInstanceIDNone) {
bad_access_key.Set("Nonpositive");
return false; return false;
}
// The embedder is trying to access an instance ID that has not yet been // The embedder is trying to access an instance ID that has not yet been
// allocated by GuestViewManager. This could cause instance ID // allocated by GuestViewManager. This could cause instance ID
// collisions in the future, and potentially give one embedder access to a // collisions in the future, and potentially give one embedder access to a
// guest it does not own. // guest it does not own.
if (guest_instance_id > current_instance_id_) if (guest_instance_id > current_instance_id_) {
bad_access_key.Set("Unallocated");
return false; return false;
}
// We might get some late arriving messages at tear down. Let's let the // We might get some late arriving messages at tear down. Let's let the
// embedder tear down in peace. // embedder tear down in peace.
...@@ -481,21 +489,28 @@ bool GuestViewManager::CanEmbedderAccessInstanceID( ...@@ -481,21 +489,28 @@ bool GuestViewManager::CanEmbedderAccessInstanceID(
return true; return true;
auto* guest_view = GuestViewBase::FromWebContents(it->second); auto* guest_view = GuestViewBase::FromWebContents(it->second);
if (!guest_view) if (!guest_view) {
bad_access_key.Set("No guest");
return false; return false;
if (guest_view->CanBeEmbeddedInsideCrossProcessFrames()) {
// MimeHandlerViewGuests (PDF) may be embedded in a cross-process frame.
return embedder_render_process_id ==
guest_view->GetOwnerSiteInstance()->GetProcess()->GetID();
} }
// MimeHandlerViewGuests (PDF) may be embedded in a cross-process frame.
// Other than MimeHandlerViewGuest, all other guest types are only permitted // Other than MimeHandlerViewGuest, all other guest types are only permitted
// to run in the main frame. // to run in the main frame or its local subframes.
return embedder_render_process_id == guest_view->owner_web_contents() const int allowed_embedder_render_process_id =
->GetMainFrame() guest_view->CanBeEmbeddedInsideCrossProcessFrames()
->GetProcess() ? guest_view->GetOwnerSiteInstance()->GetProcess()->GetID()
->GetID(); : guest_view->owner_web_contents()
->GetMainFrame()
->GetProcess()
->GetID();
if (embedder_render_process_id != allowed_embedder_render_process_id) {
bad_access_key.Set("Bad embedder process");
return false;
}
return true;
} }
GuestViewManager::ElementInstanceKey::ElementInstanceKey() GuestViewManager::ElementInstanceKey::ElementInstanceKey()
......
include_rules = [
"+components/crash/core/common/crash_key.h",
]
...@@ -14,6 +14,7 @@ ...@@ -14,6 +14,7 @@
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/strings/stringprintf.h" #include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "components/crash/core/common/crash_key.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h" #include "content/public/browser/render_process_host.h"
...@@ -270,6 +271,9 @@ bool WebViewInternalExtensionFunction::PreRunValidation(std::string* error) { ...@@ -270,6 +271,9 @@ bool WebViewInternalExtensionFunction::PreRunValidation(std::string* error) {
int instance_id = 0; int instance_id = 0;
EXTENSION_FUNCTION_PRERUN_VALIDATE(args_->GetInteger(0, &instance_id)); EXTENSION_FUNCTION_PRERUN_VALIDATE(args_->GetInteger(0, &instance_id));
// TODO(780728): Remove crash key once the cause of the kill is known.
static crash_reporter::CrashKeyString<128> name_key("webview-function");
crash_reporter::ScopedCrashKeyString name_key_scope(&name_key, name());
guest_ = WebViewGuest::From(render_frame_host()->GetProcess()->GetID(), guest_ = WebViewGuest::From(render_frame_host()->GetProcess()->GetID(),
instance_id); instance_id);
if (!guest_) { if (!guest_) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment