Since update to 5.22.0 beta Signal is crashing constantly #11569
Comments
|
Similary to me. For me it just crashes when I launch the app. Logcat:
|
|
Same. Simply just opening crashes. It never crashed before. The only thing that's changed is signal update . |
|
Same with 5.22.1 with Pixel 3 and grapheneOS : instant crash. In the Android recents menu (swipe from bottom to the middle of the screen), there should be a Signal app tile. If i clicked on it, it crashed. But sometimes during 1s, i could see the app and that there was the rating request popup. I reclicked on the signal tile in the recents menu and tried to click the "no thanks" button before it crashes. Done. The app works now. |
|
This crash has been happening to me since v5.22.0 on two GrapheneOS devices (Pixel 3a and 3a XL), both of them without Play services installed. Like other users, Signal hasn't crashed like this prior to v5.22.0
From running Logcat output for an existing installation (seems like hardened_malloc is detecting an issue when it tries to purge memory; the first line is from https://github.com/GrapheneOS/hardened_malloc/blob/39526453181babac2ba2ef3d09a406f2b7ff0305/memory.c#L92): Logcat output for a new installation: |
|
@valldrac has done research into a similar problem with Molly already. SQLCipher has completely broken usage of memory locking. They're trying to lock memory for allocations that aren't page aligned and multiples of the page size. They're also often not unlocking the memory before freeing it. They're using realloc on memory they locked despite it not preserving the locking or removing it for the freed memory. This is completely broken. It results in malloc ending up having freed pages that are memory locked where The best short term solution is likely completely disabling all the broken memory locking support until it's fixed. It's very clearly broken just from a quick glance over it. hardened_malloc is doing the right thing. We're not going to remove our memory corruption error checking code for madvise and other system calls, although we could disable it for specific broken apps like Signal via hard-wired checks for the application name. It would be very unfortunate if we had to do that because it's an awful hack. The only case we're currently doing something like that is for the Pixel 3 camera service because the drivers have use-after-free bugs. We don't need those kind of hacks for any of the newer devices and we aren't currently doing it for apps. |
|
If memory locking is now being enabled, that's probably the issue, because the implementation is thoroughly broken. I strongly recommend disabling it until it's fixed. It's possible there's another cause, but so far this has only occurred with Molly and now apparently also Signal and we've already determined the likely cause. From the
It's simply incorrect and broken to free memory that's still memory locked. At best, malloc is going to ignore the errors for madvise and leak memory. For jemalloc in AOSP and the stock OS, it isn't zeroing memory on free and sensitive data is going to end up persisting in memory longer since MADV_DONTNEED isn't working to forcefully deallocate the pages. The difference with jemalloc is that it's treating every error code from |
|
With the switch back to mainline SQLCipher in 5.22.2 (faa36d4), Signal is no longer crashing for me on 5.22.2 |
|
Was this switch turning on memory locking? Someone should check, since that's likely the cause. |
|
Working again with 5.22.2 beta. |
|
The issue is probably just going to come back if they deploy this again without addressing it though. |
|
@thestinger Do you think I should reopen it and leave it open? |
|
Yes. |
|
Waiting for the cause to get resolved. |
|
Still crashing for me on 5.22.3 |
|
@timcappalli On GrapheneOS or elsewhere? |
Pixel 5 Android 12b4.1 |
|
I think you should file a separate issue. It's unlikely to be the same problem as this one which appears to be a bug uncovered by hardened_malloc on GrapheneOS. |
|
I have also crashes but diffrent reason? I also reinstal signal with restore backup. It no helps.... (5.22.3) (Edited, I created: #11572) |
|
You should file a new issue. It's not the same problem. |
|
I have the same problem since today (5.22.5) on a Samsung Galaxy A51 with android 11: directly after starting signal it crashes instantly, it doesn't work any longer |
|
Should open a new issue. It's not the same cause as this. |
|
Thx, I have opened #11583 |
|
I have verified that the crashes happens when the memory protection is enabled, and SQLCipher does mlock after malloc, but fails to properly munlock before free. So later when the Android framework requests a memory purge, madvice fails with EINVAL for the freed but still locked pages. Signal disables the memory protection in the postKey callback function with A quick fix could be to disable SQLCipher memory protection at compile time. |
|
On GrapheneOS (Build fingerprint: 'google/sargo/sargo:11/RQ3A.210805.001.A1/2021.08.09.20:userdebug/test-keys'), the new Signal 5.23.0 beta version makes use of the SQLCipher fork again, although it's using a newer version of their fork (org.signal:android-database-sqlcipher:4.4.3-S2): fa26eb2. Signal 5.23.0 for me brings back the crashing on startup with the same error line of Here's a logcat output from when the app starts to when the app is killed: |
|
https://community.signalusers.org/t/beta-feedback-for-the-upcoming-android-5-23-release/36545/29
|
|
Hi, thanks for this bug. It's essential one to me. I've reported another similar issue #11508 I'm not entirely sure, but I started noticing the issue after Signal update when customization (backgrounds, fonts, etc.) for chats have been implemented. Maybe it would be helpful. |
|
Signal beta version 5.23.3 no longer has fatal crashes for me (on a self-compiled build). The commit in 5.23.3 that updates their SQLCIpher fork (to version 4.4.3-S3) is bb446ac. Since the commit also involves removing the |
|
Yeah, by default the new SQLCipher has memory protection off and people can instead choose to enable it. Glad it's fixed 👍 |
|
But ultimately you want the memory protection, but without the bugs that are causing it to crash on GrapheneOS. |
|
Signal has had memory protection disabled since they updated to major version 4 of SQLCipher, so it's likely they were having issues with it beforehand. |
|
The issue is that it's very slow. I believe it was 6x slower for most of our core queries. |
|
That's tied to the same issue that's causing crashes: it's not really being done properly. Memory locking works on a page level but in SQLCipher implemented as if it's reasonable to do it for individual tiny malloc allocations. It could only correctly do that if all the allocations were page-aligned and sized to multiples of the page size. It's important to remember to unlock it before freeing it though, and presumably after using MADV_DONTNEED to drop/clear the memory perhaps with That adds the overhead of a system call for allocation and another one for free. It also means realloc can't be used safely and the application has to just allocate, memory lock, copy to that, clear the old memory, unlock it and free it. Since it's currently not always remembering to unlock before freeing, it's also effectively leaking memory, since malloc can't purge with madvise and hardened_malloc just has the unique property of actually checking for errors and treating non-ENOMEM errors as serious logic errors in the program. This is actually a serious problem because it prevents dropping that memory via madvise. It could be replaced with fresh pages but it can't really be expected that malloc identifies the bug, confirms it via /proc/self/maps to make sure it's not caused by the application unmapping part of it and then it could replace the mapping. |
|
Hello @thestinger - thanks for the valuable feedback on the implementation of memory security in SQLCipher, and how it is impacting the use of Signal on Graphene. Based on some of this input and review of the implementation we have made a few changes that will be included in the next version of SQLCipher. First, the implementation of the xRealloc callback will be modified when memory security is enabled. This will avoid the issue you identified where a realloc operation occurs and does not properly munlock the segment. While not as efficient the new implementation uses the same approach you described, allocating a new segment, copying to it, and then unlocking and freeing the old one. Based on this input we also identified a separate issue where, due to the locking being on by default, segments could be locked early in the connection lifecycle, and then if locking is subsequently disabled, those segments could be freed without unlocking. After reviewing all the options, we have made the decision to turn the memory security feature off be default. An application can then turn it on, but will not be able to turn it off after doing so. This ensures the consistent tracking of allocations guaranteeing that munlock will be called before free on any mlocked allocations. Finally, we do recognize that, in terms of efficiency, the current implementation can result in multiple mlock / munlock cycles. Unfortunately we are somewhat constrained with the implementation in the memory management plugin architecture for SQLite. We will review this in the future to see if there are further opportunities for optimization. In the mean time though, in our testing these changes have eliminated the crash issues observed on Graphene, both with memory security enabled and disabled. Thanks again for your input on this issue, and let us know if you have any other suggestions or comments. |
Bug description
After updating Signal to 5.22.0 beta Signal is crashing right after opening it or few seconds later. Sending a debug log is not possible. I stil get SMS notifications but not Signal messages ones.
Steps to reproduce
Device info
Device: Pixel 3
Android version: 11 (GrapheneOS with sandboxed Play Services compatibility layer)
Signal version: 5.22.0 beta
The text was updated successfully, but these errors were encountered: