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

Issue 718814 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression

Blocking:
issue 721668



Sign in to add a comment

Regression : Chrome Crash is seen on entering 5/6/7 characters in 'Password' text box of any wi-Fi network

Project Member Reported by mmanchala@chromium.org, May 5 2017

Issue description

Chrome Version: 60.0.3086.3/9517.1.0  dev-channel Candy,Daisy and Minnie
OS: Chrome

What steps will reproduce the problem?
(1)Sign into User -> Click on Uber Tray
(2)Now select Network option ->Try to click on any new network -> Enter 5/6/7  characters in 'Password' text box and click on 'Connect button
(3)Now observe Chrome Crash (Please refer video)

Expected: No Crash should be seen on entering 5/6/7 characters in 'Password' text box. It should try to connect to network and if Password is wrong 'Network Connection Error' Notification should be seen

Actual: Instead Chrome crash is seen

This is Regression issue as same is working fine in 59.0.3071.33/9460.20.0 dev-channel Candy

@tbuckley : please confirm the Issue

Below is the Crash id:
6eb10923b0000000

Stack Trace:
Thread 4 CRASHED [SIGABRT @ 0x000003e8000008ad ] MAGIC SIGNATURE THREAD
Stack Quality
100%Show frame trust levels
0x00007ef041165eb2
(libc-2.23.so -raise.c:54)
raise
0x00007ef041167cd5
(libc-2.23.so -abort.c:89)
abort
0x00005a87e5737064
(chrome+ 0x02c3e064)
base::debug::BreakDebugger()
0x00005a87e574a42c
(chrome+ 0x02c5142c)
logging::LogMessage::~LogMessage()
0x00005a87e61a1be3
(chrome+ 0x036a8be3)
dbus::Bus::OnConnectionDisconnectedFilter(DBusConnection*, DBusMessage*, void*)
0x00007ef04273e445
(libdbus-1.so.3.14.8 -dbus-connection.c:4709)
dbus_connection_dispatch
0x00005a87e61a2898
(chrome+ 0x036a9898)
dbus::Bus::ProcessAllIncomingDataIfAny()
0x00005a87e3a3fbb6
(chrome+ 0x00f46bb6)
base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*)
0x00005a87e3a323c2
(chrome+ 0x00f393c2)
base::MessageLoop::RunTask(base::PendingTask*)
0x00005a87e3a335a8
(chrome+ 0x00f3a5a8)
base::MessageLoop::DoWork()
0x00005a87e3a33dcb
(chrome+ 0x00f3adcb)
base::MessagePumpLibevent::Run(base::MessagePump::Delegate*)
0x00005a87e576e1af
(chrome+ 0x02c751af)
base::RunLoop::Run()
0x00005a87e578e907
(chrome+ 0x02c95907)
base::Thread::ThreadMain()
0x00005a87e5789d0c
(chrome+ 0x02c90d0c)
base::(anonymous namespace)::ThreadFunc(void*)
0x00007ef0424e7557
(libpthread-2.23.so -pthread_create.c:333)
start_thread
0x00007ef04122908c
(libc-2.23.so+ 0x000f708c)
clone


 
Actual_Crash.mp4
8.0 MB View Download
Expected.mp4
13.1 MB View Download
Below are the few other Crash ids from different devices:

bfd2e65e80000000
82b0ad9e80000000
f046ed9e80000000

Comment 2 by ajha@chromium.org, May 5 2017

Cc: moh...@chromium.org josa...@google.com
Though the stack trace is completely diff from  Issue 718411 (fixed on 60.0.3086.3/9517.1.0) but based on somewhat similar manual repro scenario cc'ing mohsen@ for more inputs.
Cc: dhadd...@chromium.org harpreet@chromium.org
Owner: moh...@chromium.org
yeah, stack trace is different but it appears to be related to the change landed for  issue 718411  since this crash was not in 3086.0 and the only change was the fix for above bug 
https://chromium.googlesource.com/chromium/src/+log/60.0.3086.0..60.0.3086.3?pretty=fuller&n=10000

mohsen@ can you evaluate?
Components: -Blink>Network
Removing Blink>Network which is about web APIs (XHR, WebSocket, etc.)
Cc: steve...@chromium.org
Owner: josa...@chromium.org
That doesn't seem related to my UI change, rather looks like an issue in the networking stack. The reason you weren't seeing this crash before my change is, probably, that the crash I fixed didn't let the code reach this crash! Now the first crash is not happening, so you see the second one.
Cc: derat@chromium.org cernekee@chromium.org benchan@chromium.org
I can reproduce this and strangely I am getting the same error as we have been seeing in 714547:

"D-Bus connection was disconnected. Aborting."

That is preceeded by a "SetProperties: Failed: Config.SetProperties Failed" error, which is expected.

So I don't think this is related to any of the status area changes, but rather some change is causing certain types of DBus errors to trigger this CHECK:

https://cs.chromium.org/chromium/src/dbus/bus.cc?q=bus.cc+package:%5Echromium$&dr&l=1189

Comment 8 by derat@chromium.org, May 8 2017

Cc: hashimoto@chromium.org satorux@chromium.org
Yeah, this reminds me of issue 714547 too.

Cc-ing some people who might have ideas about how other D-Bus errors could trigger a (bogus?) "Disconnected" error.
I can reproduce this in chrome @3086.0 (#468250).

So, either this is due to a cros change, or it is from an older chrome change and has not been reported.

Also reproducible at chrome @3085.0 (#468217).

Looking at where this is failing, it seems like it might be a symptom of a more serious dbus failure related to a chromeos change?

Can we confirm where this crash was introduced? If the system tray crashes are getting in the way of the password dialog showing, it should be possible to trigger the dialog from the Settings UI instead.

Well, the system tray crash ( issue 718411 ) should not happen before password dialog. They happen during the connecting animation. The password dialog closes the system tray, so no animation should be kicked off unless the system menu is re-opened while connecting. So, those crashes should not interfer with this one and last part of my comment in #6 is not valid!
https://codereview.chromium.org/2808983002/ could be related? does this issue reproduce with the change reverted locally?
That was the first thing I tried :) Didn't help.

I've just commented here crbug.com/714547#c14 but I'm guessing that issue 714547 and this issue are a reappearance of  issue 181655 .  The stack trace at #c10 also included dbus::Bus::OnConnectionDisconnected() and dbus::Bus::OnConnectionDisconnectedFilter ().

In short, empty object paths somehow caused Chrome getting "disconnected" signal. The fix at that time was to remove --disable-check from dbus's build config.

https://chromium-review.googlesource.com/c/45545/5/sys-apps/dbus/dbus-1.6.8-r2.ebuild

Owner: wonderfly@chromium.org
Ah, and it looks like we recently updated the dbus version to 1.10.12 :

https://chromium-review.googlesource.com/c/461717/

Can we try removing that flag in the updated ebuild?

Can we add PRESUBMIT check or something to prevent --disable-check (now --disable-checks?) from getting added again?
Forgot to mention but passing empty object paths sounds like a bug. We should investigate where these come from and fix these.
Cc: wonderfly@chromium.org vapier@chromium.org
Owner: ----
I don't have a particular reason for why I added back the --disable-checks flag during the dbus upgrade, probably just copied from gentoo upstream.

The CL that removed it long ago was saying, "Let me remove this flag until fixing the actual root cause", so is the real bug still alive?

Anyways, I don't have an objection of removing it again. Just that I might not be the best person to properly test it though - only work on chromium os occasionally.
Apparently --disable-checks came from upstream ebuild.
https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-apps/dbus/dbus-1.10.12.ebuild

If the empty object path is the root cause and the sender is using libchrome's D-Bus library, we can add a CHECK in dbus::MessageWriter to prevent sending empty ObjectPath.
Owner: hashimoto@chromium.org
Status: Started (was: Assigned)
Made a CL to remove --disable-checks.
https://chromium-review.googlesource.com/c/499967/
Cc: aashuto...@chromium.org
Project Member

Comment 22 by bugdroid1@chromium.org, May 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/4afb4364174e7491eddd6cf757f35446acacd8c7

commit 4afb4364174e7491eddd6cf757f35446acacd8c7
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Thu May 11 03:58:08 2017

sys-apps/dbus: Remove --disable-check

Building dbus with --disable-check results in mysterious behaviors when
invalid values (e.g. empty object path) are sent on D-Bus.

BUG= chromium:718814 
TEST=emerge-$BOARD dbus

Change-Id: I53572b0d238279afb9289a19d1c60e3f2e77bbf0
Reviewed-on: https://chromium-review.googlesource.com/499967
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Satoru Takabayashi <satorux@google.com>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/4afb4364174e7491eddd6cf757f35446acacd8c7/sys-apps/dbus/dbus-1.10.12.ebuild
[rename] https://crrev.com/4afb4364174e7491eddd6cf757f35446acacd8c7/sys-apps/dbus/dbus-1.10.12-r2.ebuild

Owner: steve...@chromium.org
Status: Assigned (was: Started)
Now we are no longer using --disable-check when building dbus, so invalid values like empty object paths don't result in mysterious behavior like chrome crash.

Steven, is it possible to locate the actual code which is sending invalid values with D-Bus (e.g. empty object paths, non-UTF8 strings)?
Crash should be gone, but we may end up repeating this again when uprevving libdbus in future.
Owner: hashimoto@chromium.org
I don't know a good way to identify that off the top of my head. I didn't see any indication that Chrome was sending bad info when I investigated this, so I wonder if it was caused by Shill sending empty object paths in a response when an error occurred (but I'm not at all certain that was the case).

I think we should resolve this bug is fixed and maybe file a separate bug to investigate that?

Blocking: 721668
Status: Fixed (was: Assigned)
OK, filed issue 721668 for investigation.

We shouldn't see this crash no longer, closing as fixed.
Status: Verified (was: Fixed)
No crash observed on 9563.0.0, 60.0.3102.0 with repro steps in bug description.
Project Member

Comment 28 by bugdroid1@chromium.org, Jun 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/005ac9903d0e8cd98281496f48bc1b3296b3769b

commit 005ac9903d0e8cd98281496f48bc1b3296b3769b
Author: Ryo Hashimoto <hashimoto@google.com>
Date: Wed Jun 28 10:09:52 2017

sys-apps/dbus: Add a presubmit check against --disable-checks

With --disable-checks, dbus daemon behaves in an unpredictable way.
We should avoid adding this again when uprevving dbus.

BUG= chromium:718814 
TEST=repo upload

Change-Id: Ic84be1c6428243e84b5f55685e601f50f573796c
Reviewed-on: https://chromium-review.googlesource.com/505790
Commit-Ready: Ryo Hashimoto <hashimoto@chromium.org>
Tested-by: Ryo Hashimoto <hashimoto@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>

[add] https://crrev.com/005ac9903d0e8cd98281496f48bc1b3296b3769b/sys-apps/dbus/presubmit_check.sh
[modify] https://crrev.com/005ac9903d0e8cd98281496f48bc1b3296b3769b/PRESUBMIT.cfg

Sign in to add a comment