New issue
Advanced search Search tips

Issue 642241 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

SessionManagerClientImpl::CallRestartJobWithValidFd may crash on a D-Bus error

Project Member Reported by hashimoto@chromium.org, Aug 30 2016

Issue description

https://codereview.chromium.org/2277803005/ introduced a new function which looks like this:

  void HandleDBusError(dbus::ErrorResponse* response) {
    LOG(ERROR) << "DBus error " << response->ToString();
  }

This may crash because ObjectProxy::CallMethodWithErrorCallback can pass a nullptr to the error callback when an error response is not available (i.e. an error happening locally because of a bad D-Bus connection.)
Also, IIUC this logging is unnecessary because ObjectProxy::CallMethod calls ObjectProxy::LogMethodCallFailure on an error.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 31 2016

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

commit b159d73f7098b1d36aff32c4ff593495a3c710b7
Author: hashimoto <hashimoto@chromium.org>
Date: Wed Aug 31 09:20:58 2016

chromeos: Fix potential crash in SessionManagerClient

ErrorResponse can be null when an error happens locally.
Also, this logging is unnecessary as ObjectProxy::CallMethod is already
doing that.

BUG= 642241 

Review-Url: https://codereview.chromium.org/2291453006
Cr-Commit-Position: refs/heads/master@{#415591}

[modify] https://crrev.com/b159d73f7098b1d36aff32c4ff593495a3c710b7/chromeos/dbus/session_manager_client.cc

Status: Fixed (was: Started)

Comment 4 by dchan@chromium.org, Oct 7 2016

Labels: VerifyIn-55

Comment 5 by dchan@google.com, Nov 19 2016

Labels: VerifyIn-56

Comment 6 by dchan@google.com, Jan 21 2017

Labels: VerifyIn-57

Comment 7 by dchan@google.com, Mar 4 2017

Labels: VerifyIn-58

Comment 8 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 9 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Labels: VerifyIn-61

Comment 11 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment