New issue
Advanced search Search tips

Issue 875993 link

Starred by 3 users
Status: Started
Owner:
pmonette@chromium.org
Cc:
chrisha@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

A dead man switch is needed for the third-party modules blocking feature

Project Member Reported by pmonette@chromium.org, Aug 20 
In some circumstances, blocking a module will cause Chrome to crash and restart causing an infinite loop because the blocking feature will not turn off automatically.

One of the way this can happen is by binary planting a DLL with the same name of one of Chrome's dependencies. (see details here: https://crbug.com/870463)

While this specific case is now fixed, it's important that we have a mechanism in place to avoid these type of situations.

Comment 1 by pmonette@chromium.org, Aug 20

Labels: Merge-Request-69
This was fixed in this CL:

https://chromium-review.googlesource.com/c/chromium/src/+/1180267

Need a merge to m69.

This fixes a possible infinite crash loop. The fix was confirmed to be working on canary.

The merge would be very safe given the simplicity and the size of the change (5 characters).
Project Member

Comment 2 by sheriffbot@chromium.org, Aug 20

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the 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

Comment 3 by gov...@chromium.org, Aug 20

Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #1. Pls merge ASAP. Thank you.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 20

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

commit 87dd0df4f02fa02bbbaf1a5ea40ce91f0097dd1a
Author: Penny MacNeil <pennymac@chromium.org>
Date: Mon Aug 20 20:04:25 2018

[chrome_elf] Temporarily still listen to the old beacon.

Don't initialize third_party_dlls if the beacon says not to.

Bug: 875993
Test: chrome_elf_unittests.exe
Change-Id: I5334d82eb11ecfcc026ed1bfaf25a02070830e59
Reviewed-on: https://chromium-review.googlesource.com/1180267
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Penny MacNeil <pennymac@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#584227}(cherry picked from commit 6472da3fbb75d4b582b6daf19ecb5dbf85037e04)
Reviewed-on: https://chromium-review.googlesource.com/1182062
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#722}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/87dd0df4f02fa02bbbaf1a5ea40ce91f0097dd1a/chrome_elf/chrome_elf_main.cc

Comment 5 by pnangunoori@chromium.org, Aug 23

Labels: Needs-Feedback
pmonette@ -- Could you please provide the manual steps so that we can verify the fix from TE's end and add TE-Verified labels.

Thanks!

Comment 6 by pmonette@chromium.org, Aug 23

Labels: -Target-69 Target-70
Bumping this to m70 since another bug (870998) is preventing us from releasing the blocking feature.

Comment 7 by gov...@chromium.org, Aug 23 

Per comment #6, is the change merged to M69 at #4 need to be reverted?

Comment 8 by pmonette@chromium.org, Aug 23 

It doesn't need to be reverted. It's not causing any harm and it is still only affecting a feature behind finch.

Comment 9 by benhenry@google.com, Jan 9

Labels: -Pri-0 Pri-1
I'm bumping this down to P-1, as it's no longer an emergency.
►

Sign in to add a comment