Closed Bug 1482122 Opened 2 years ago Closed 2 years ago
Crash in ns
TArray _Impl<T>::Clear | wcsrtombs | mozilla::jni::detail::Proxy Native Call<T>::operator()
This bug was filed from the Socorro interface and is report bp-8f7aae82-8063-4131-9ebe-f0ae30180809. ============================================================= Seen while looking at Android crash stats on release: https://bit.ly/2MumaYo. Not sure where to bucket it, but it appears many of the signatures are potential UAFs so marking security sensitive. Only one crash seen in b62 so far. Unfortunately comments are not useful. Top 10 frames of crashing thread: 0 libxul.so nsTArray_Impl<BroadcastListener*, nsTArrayInfallibleAllocator>::Clear 1 libxul.so wcsrtombs 2 libxul.so mozilla::jni::detail::ProxyNativeCall<mozilla::GeckoScreenOrientation, mozilla::java::GeckoScreenOrientation, true, false, short, short>::operator widget/android/jni/Natives.h:418 3 libxul.so mozilla::detail::RunnableFunction<mozilla::jni::detail::ProxyNativeCall<mozilla::GeckoScreenOrientation, mozilla::java::GeckoScreenOrientation, true, false, short, short> >::Run xpcom/threads/nsThreadUtils.h:552 4 libxul.so nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1090 5 libc.so libc.so@0x1956b 6 libxul.so wcsrtombs 7 libxul.so mozilla::detail::VariantImplementation<unsigned char, 0, const int, const char*, void > > mfbt/Variant.h:219 8 libxul.so mozilla::PerformanceCounter::GetDispatchCount android-ndk/sources/cxx-stl/llvm-libc++/include/atomic:885 9 libxul.so wcsrtombs =============================================================
Group: core-security → firefox-core-security
Product: Core → Firefox for Android
Version: 61 Branch → Trunk
It seems like this may be a variant of bug 1476250. What do you think, :gsvelto?
Flags: needinfo?(snorp) → needinfo?(gsvelto)
It could be, most of the stack seems garbled except for the part about calling OnOrientationChange(). The crashes look like UAFs alright since >80% have an e5e5e5e5 value as the crashing address and this would be consistent with bug 1476250 which is basically the same problem: the last screen orientation observer is unregistered, the observer array is deleted and then something tries to touch. There's something important here I hadn't noticed in the other bug: the crashing thread is _not_ the main thread and that's just wrong because the HAL shouldn't be accessed from other threads. I'll first have to check where the JNI code is calling OnOrientationChange() because if it's not from the main thread then that should be sorted out first.
Scratch what I wrote before, the notification is sent on the correct thread. I have a hard time figuring out how the race can possibly happen. The patch that I'm writing for bug 1476250 should address this issue too though by never destroying the list of observers as long as the service is in use thus entirely preventing the race.
Updating assignee based on comment 4. Thanks!
Assignee: nobody → gsvelto
The fix for this issue is undergoing review in bug 1476250 where I had already started working on it before this bug was filed.
I'm about to land the patch for bug 1476250, it should fix this too.
Status: NEW → ASSIGNED
There have been no instances of this crash for versions including the fix in bug 1476250, closing as fixed.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Tested with the following devices on 63.0b13: - Motorola Nexus 6(Android 7.1.1) - Nexus 9(Android 6.0.1) - Sony Xperia Z5 Premium(Android 6.0.1) - Xiaomi Mi4i(Android 5.0.2) We didn't see the crash while performing the steps from bug 1476250.
Depends on: 1476250
Whiteboard: [adv-main63+] → [adv-main63+] fixed by bug 1476250
You need to log in before you can comment on or make changes to this bug.