Crash in ImeController::CanCycleIme |
||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=6071399941931008 Fuzzer: meacer_chromebot_extensions Job Type: linux_asan_chrome_chromeos Platform Id: linux Crash Type: UNKNOWN Crash Address: 0x000000000000 Crash State: ImeController::CanCycleIme ash::AcceleratorController::AcceleratorPressed ui::AcceleratorManager::Process Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=458162:458196 Reproducer Testcase: https://clusterfuzz.com/download/AMIfv96PKwTwtmoXrhPY_QGn7GmNnPsaj9VThGck4hc7A6JcVO1Z92O3UYVG2SRbEBI8mP54oxJXPmL9wOeaZgZ1aC9T2mseMoLwj0nIbN6_U4AhFBpP6zHPuCt-BQqaSELZitv1H6sLrHMX9TlWNZhPlp7gX-y01wM8YvQ4VRRwci3s1hqOtCAiViQzSmxJKnkdNI1-MSOSkHHdgMzbW5_GJuJanBV4wg3106kmcRaD1a5SoxSr3QpjEt_pCuXGrwpiVbmuM49mwUFDId3TKTMZ9CClmJW3w5TvMgThujEffekC2WAKX2h7RQ4T5kS-J0g7NxlZBwb_CSptiNoQbqZhHGSbf66oqYKtw9EeQLKBXWtPRL29oO5sBm4DCLAdL_R_zyQDLR0fHSKe9YPV-VlssWhh7iSVfA?testcase_id=6071399941931008 Additional requirements: Requires Gestures Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Mar 22 2017
+ShowMessageBoxImpl owners. What happens is ExtensionErrorReporter::ReportLoadError() . Which turns into ShowMessageBox and then crashes in ShowMessageBoxImpl because it will run the same message loop during initialization. +msw, +pkasting, what do you think? I don't think running event loop is safe there. Received signal 11 SEGV_MAPERR 000000000000 #0 0x7fd51ef922d1 __interceptor_backtrace #1 0x7fd526511f7e base::debug::StackTrace::StackTrace() #2 0x7fd526510e1d base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7fd5178ea340 <unknown> #4 0x7fd52e5ecd80 ImeController::CanCycleIme() #5 0x7fd52d63f106 ash::AcceleratorController::AcceleratorPressed() #6 0x7fd5352eb6af ui::AcceleratorManager::Process() #7 0x7fd5362fae9c wm::AcceleratorFilter::OnKeyEvent() #8 0x7fd528b46697 ui::EventDispatcher::DispatchEventToEventHandlers() #9 0x7fd528b45463 ui::EventDispatcher::ProcessEvent() #10 0x7fd528b44f7b ui::EventDispatcherDelegate::DispatchEvent() #11 0x7fd5352fc0fb ui::EventProcessor::OnEventFromSource() #12 0x7fd52d804913 ash::AshWindowTreeHostX11::DispatchKeyEventPostIME() #13 0x7fd535d6e1a0 ui::InputMethodBase::DispatchKeyEventPostIME() #14 0x7fd535d73663 ui::InputMethodChromeOS::DispatchKeyEvent() #15 0x7fd535d744b9 ui::InputMethodChromeOS::DispatchKeyEvent() #16 0x7fd52d806bf1 ash::InputMethodEventHandler::OnKeyEvent() #17 0x7fd528b46697 ui::EventDispatcher::DispatchEventToEventHandlers() #18 0x7fd528b45463 ui::EventDispatcher::ProcessEvent() #19 0x7fd528b44f7b ui::EventDispatcherDelegate::DispatchEvent() #20 0x7fd5352fc0fb ui::EventProcessor::OnEventFromSource() #21 0x7fd528b4912c ui::EventSource::SendEventToProcessor() #22 0x7fd535f7fee1 aura::WindowTreeHostX11::DispatchEvent() #23 0x7fd528b1ecdb ui::PlatformEventSource::DispatchEvent() #24 0x7fd528c4b9c0 ui::X11EventSource::ExtractCookieDataDispatchEvent() #25 0x7fd528c4b70c ui::X11EventSource::DispatchXEvents() #26 0x7fd528c4afbd ui::(anonymous namespace)::XSourceDispatch() #27 0x7fd51a700ce5 g_main_context_dispatch #28 0x7fd51a701048 <unknown> #29 0x7fd51a7010ec g_main_context_iteration #30 0x7fd5265895a4 base::MessagePumpGlib::Run() #31 0x7fd52657e989 base::MessageLoop::RunHandler() #32 0x7fd52660cd5e base::RunLoop::Run() #33 0x7fd52e0a8e62 chrome::(anonymous namespace)::ShowMessageBoxImpl() #34 0x7fd52e0a8272 chrome::ShowWarningMessageBox() #35 0x7fd52c4801d7 ExtensionErrorReporter::ReportError() #36 0x7fd52c47f33b ExtensionErrorReporter::ReportLoadError() #37 0x7fd52c58e491 extensions::UnpackedInstaller::ReportExtensionLoadError() #38 0x7fd52c58ddb1 extensions::UnpackedInstaller::LoadFromCommandLine() #39 0x7fd52c4af9f7 ExtensionService::LoadExtensionsFromCommandLineFlag() #40 0x7fd52c4aee88 ExtensionService::Init() #41 0x7fd52c4e70e1 extensions::ExtensionSystemImpl::Shared::Init() #42 0x7fd52c4e81b5 extensions::ExtensionSystemImpl::InitForRegularProfile() #43 0x7fd5259d333d ProfileManager::DoFinalInitForServices() #44 0x7fd5259d2d6b ProfileManager::DoFinalInit() #45 0x7fd5259d6112 ProfileManager::AddProfile() #46 0x7fd5259c2556 ProfileManager::CreateAndInitializeProfile() #47 0x7fd5259c1aff ProfileManager::GetProfile() #48 0x7fd5259c12bc ProfileManager::GetActiveUserOrOffTheRecordProfileFromPath() #49 0x7fd5259c16db ProfileManager::GetActiveUserProfile() #50 0x7fd52578f846 ChromeBrowserMainParts::PreMainMessageLoopRunImpl() #51 0x7fd52578dcc6 ChromeBrowserMainParts::PreMainMessageLoopRun() #52 0x7fd5241960a3 chromeos::ChromeBrowserMainPartsChromeos::PreMainMessageLoopRun() #53 0x7fd52044499f content::BrowserMainLoop::PreMainMessageLoopRun() #54 0x7fd52110d9ad content::StartupTaskRunner::RunAllTasksNow() #55 0x7fd52043e6b2 content::BrowserMainLoop::CreateStartupTasks() #56 0x7fd5204530b3 content::BrowserMainRunnerImpl::Initialize() #57 0x7fd520435601 content::BrowserMain() #58 0x7fd5256160af content::ContentMainRunnerImpl::Run() #59 0x7fd525611612 content::ContentMain() #60 0x7fd51f01b9f2 ChromeMain
,
Mar 22 2017
Showing a message box has to spin an event loop, or it can't detect the button press. Why is this crashing when it's in a nested message loop?
,
Mar 22 2017
> Why is this crashing when it's in a nested message loop? I think this is because ProfileManager::CreateAndInitializeProfile() (incl. everything that is called from it) doesn't expect the run loop to spin while it is running. Could we do PostTask(Bind(&ShowMessageBoxImpl)) inside ShowWarningMessageBox() instead of direct call?
,
Mar 22 2017
Maybe; we could also do something like using a different surface (e.g. an infobar). One of the things I'm worried about is other message box users getting into similar trouble, so I wouldn't want to just fix by avoiding the call here if some other caller will trip the same bug. What part of CreateAndInitializeProfile() is breaking with this? Basically, what I'm really wanting to know is precisely what kind of unsafe data manipulation (or whatever) is occurring here, in detail.
,
Mar 22 2017
That's a tricky question as it is difficult to traverse a whole call tree inside CreateAndInitializeProfile(). I think there are numerous cases. The stack trace above suggests that we cannot handle user input events until profile is fully initialized. One of the reasons is that IME is implemented as extension, so it cannot be used until extension initialization code is completed. We also cannot handle user input events because they should be targeted to the active user, but no active user exists before its profile is initialized (for example, we need mapping User <-> Profile for many ChromeOS services). I also see that Profile attributes are initialized after profile has been added to the storage, and thus "ephemeral" flag may not be set for the profile.
,
Mar 22 2017
I guess I was more hoping to know what the null pointer near CanCycleIme() was about. Sounds like maybe the issue is that extension init hasn't actually finished yet, so we can't safely access extensions, but the accelerator handling code assumes it can access the IME, which is implemented as an extension. If that's correct, it's hard to know whether there's something wrong with that (for example, having IME be an extension at all sounds suspicious to me), but it may be reasonable to post a task to show this message box, rather than doing it synchronously.
,
Mar 22 2017
...That said, I kinda wonder whether all warning message boxes shouldn't be shown off posted tasks in that case, for safety's sake? Are there any call sites that want to show a warning and then go ahead and do something that we really need to show the warning in advance of doing?
,
Mar 22 2017
> I guess I was more hoping to know what the null pointer near CanCycleIme() was about. It is a pointer to current state of Input Methods Engine. To create one we need IME initialized. Side-note: Actually IME is a very specific extension. It is component extension and thus built-in. But it still requires initialization (and some times it depends on external files). Also, the API is extension-based, so we need extensions initialized before using it. That is why IME::State is bound to the user profile. Also I remember other issues with running loops inside CreateProfile (sometimes it leads to recursive CreateProfile). So I would prefer to never display messaged inside profile initialization. We can either implement a delayed error messages in extensions initialization, or to always run error popup as separate task via PostTask(THREAD_UI). I think that PostTask in ShowWarningMessageBox() is simpler ;) But I am not aware of other usage scenarios of ShowWarningMessageBox() that may require existing behavior.
,
Mar 22 2017
Right, we'd have to audit if we wanted to make this change for all callers.
,
Apr 5 2017
We need this fixed. There are more crashes like this, see Issue 696329 for example.
,
Apr 5 2017
,
Apr 5 2017
I don't have bandwidth to work on this myself. My hope above was to ask the right questions to guide you to the right design for a fix. If you can't implement this, please consult an appropriate PM to get help scheduling the work with someone.
,
Apr 5 2017
,
Apr 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f8c865910c21c379df39eb057530e13630d2f00d commit f8c865910c21c379df39eb057530e13630d2f00d Author: wzang <wzang@chromium.org> Date: Fri Apr 28 01:36:26 2017 Check IME state in ImeController ImeController::CanCycleIme() and ImeController::CanSwitchIme() should first check if IME state is NULL and if it is, ignore the cycle / switch action because it does not make sense before IME initialization. BUG= 703510 Review-Url: https://codereview.chromium.org/2806613002 Cr-Commit-Position: refs/heads/master@{#467804} [modify] https://crrev.com/f8c865910c21c379df39eb057530e13630d2f00d/chrome/browser/ui/ash/ime_controller_chromeos.cc
,
Apr 28 2017
,
Apr 28 2017
ClusterFuzz has detected this issue as fixed in range 467762:467817. Detailed report: https://clusterfuzz.com/testcase?key=6071399941931008 Fuzzer: meacer_chromebot_extensions Job Type: linux_asan_chrome_chromeos Platform Id: linux Crash Type: UNKNOWN Crash Address: 0x000000000000 Crash State: ImeController::CanCycleIme ash::AcceleratorController::AcceleratorPressed ui::AcceleratorManager::Process Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=458162:458196 Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=467762:467817 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6071399941931008 Additional requirements: Requires Gestures See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by mummare...@chromium.org
, Mar 22 2017Labels: Test-Predator-Wrong M-59