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

Issue 866199 link

Starred by 1 user
Status: Fixed
Owner:
thestig@chromium.org
Closed: Jul 23
Cc:
█ xunji...@chromium.org
rmcelrath@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

NetworkConnectionTracker still has observers at shutdown due to privet code

Project Member Reported by thestig@chromium.org, Jul 20 
I was doing a manual test of the PrivetTrafficDetector functionality with gcp-cups-connector in local mode. At shutdown, Chromium failed with:

[166175:166175:0720/152255.642290:FATAL:observer_list_threadsafe.h(153)] Check failed: observers_.empty(). 
#0 0x7f5a32ea644d base::debug::StackTrace::StackTrace()
#1 0x7f5a32bb338c base::debug::StackTrace::StackTrace()
#2 0x7f5a32c223da logging::LogMessage::~LogMessage()
#3 0x7f5a2aef58ad base::ObserverListThreadSafe<>::AssertEmpty()
#4 0x7f5a2aef4689 content::NetworkConnectionTracker::~NetworkConnectionTracker()

It looks like r574700 needed more calls to PrivetTrafficDetector::Stop().
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 23 

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

commit 957f866abf50ff69a919bc2db001e326e142cf38
Author: Lei Zhang <thestig@chromium.org>
Date: Mon Jul 23 19:28:59 2018

Always stop PrivetTrafficDetector.

Currently, there are some code paths in PrivetNotificationService where
PrivetTrafficDetector is not stopped. This causes PrivetTrafficDetector
to forget to unregister itself from NetworkConnectionTracker.

BUG= 866199 

Change-Id: I75721603889a6886b012d439bd6e8f2266083ee1
Reviewed-on: https://chromium-review.googlesource.com/1145888
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577220}
[modify] https://crrev.com/957f866abf50ff69a919bc2db001e326e142cf38/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/957f866abf50ff69a919bc2db001e326e142cf38/chrome/browser/printing/cloud_print/privet_notifications.h

Comment 2 by thestig@chromium.org, Jul 23

Labels: M-69
Status: Fixed (was: Assigned)
This affects M69 and has the potential to cause UAFs in the browser process. I'll request a merge tomorrow.

Comment 3 by thestig@chromium.org, Jul 24

Labels: Merge-Request-69
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 25

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

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

Comment 5 by bugdroid1@chromium.org, Jul 25

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1473eddd1ad18653f80cd3f2f93794f4feb2b922

commit 1473eddd1ad18653f80cd3f2f93794f4feb2b922
Author: Lei Zhang <thestig@chromium.org>
Date: Wed Jul 25 22:51:04 2018

M69: Always stop PrivetTrafficDetector.

Currently, there are some code paths in PrivetNotificationService where
PrivetTrafficDetector is not stopped. This causes PrivetTrafficDetector
to forget to unregister itself from NetworkConnectionTracker.

BUG= 866199 
TBR=thestig@chromium.org

(cherry picked from commit 957f866abf50ff69a919bc2db001e326e142cf38)

Change-Id: I75721603889a6886b012d439bd6e8f2266083ee1
Reviewed-on: https://chromium-review.googlesource.com/1145888
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#577220}
Reviewed-on: https://chromium-review.googlesource.com/1150813
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#92}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/1473eddd1ad18653f80cd3f2f93794f4feb2b922/chrome/browser/printing/cloud_print/privet_notifications.cc
[modify] https://crrev.com/1473eddd1ad18653f80cd3f2f93794f4feb2b922/chrome/browser/printing/cloud_print/privet_notifications.h
►

Sign in to add a comment