Heap Corruption detected by dlmalloc_real

[fmurer]

Hi,
I am using your library in a project where I need to sign stuff and verify signatures. Sometimes it works correctly, but a lot of the time the app crashes because of a heap corruption.

D/libc-fault-recovery-native: M=0xb6fbc0f4 B=0xb6fbc174 F=0x0 P=0xb9aca908 I=11
D/libc-fault-recovery-native: FIND target F=0xb9c6c330
D/libc-fault-recovery-native: M=0xb6fbc0f4 B=0xb6fbc134 F=0x0 P=0xb9d29f90 I=3
D/libc-fault-recovery-native: FIND target F=0xb9b56750
I/Choreographer: Skipped 201 frames!  The application may be doing too much work on its main thread.
E/Typeface: SANS_LOC file not found.
W/IInputConnectionWrapper: reportFullscreenMode on inactive InputConnection
E/Typeface: SANS_LOC file not found.
D/libc-fault-recovery-native: M=0xb6fbc0f4 B=0xb6fbc1c4 F=0x0 P=0xb9cd6f58 I=21
D/libc-fault-recovery-native: FIND target F=0x0
A/libc: heap corruption detected by dlmalloc_real
A/libc: Fatal signal 6 (SIGABRT), code -6 in tid 9318
W/libc: Security Level: (1), Debug inforamtion is controlled by the DUMPABLE flag.
Application terminated.

These are the last few messages in the console. Any idea how to solve this?

[twystd]

It sounds possibly like JNI may be not releasing resources correctly.

Can you provide any more information (or preferably sample code) ?

[headuck]

I have stumbled across a few such error reports in Google play, which seems to occur to a small number of users at random and on Android 6 only, only after adding this library and calling only cryptoSignOpen. The failure occurs at various unrelated locations from the stack trace, which indicates that the problem may be hidden somewhere else. One variety of the trace:

  #00  pc 0000000000042d50  /system/lib/libc.so (tgkill+12)
 
  #01  pc 0000000000041ef1  /system/lib/libc.so (pthread_kill+32)
  #02  pc 000000000001bb4f  /system/lib/libc.so (raise+10)
  #03  pc 0000000000018d01  /system/lib/libc.so (__libc_android_abort+34)
  #04  pc 00000000000168c0  /system/lib/libc.so (abort+4)
  #05  pc 000000000001a763  /system/lib/libc.so (__libc_fatal+16)
  #06  pc 000000000002e2c1  /system/lib/libc.so (__bionic_heap_corruption_error+8)
  #07  pc 0000000000030525  /system/lib/libc.so (dlfree+308)
  #08  pc 0000000000003887  /data/app/com.headuck.headuckblocker.dev-1/lib/arm/libtweetnacl.so (Java_za_co_twyst_tweetnacl_TweetNaCl_jniCryptoSignOpen+246)
 #09  pc 0000000000573e9d  /data/app/com.headuck.headuckblocker.dev-1/oat/arm/base.odex

I am not familiar with jni, but I notice that the release function at tweetnacl-jni.c has memset called on bytes after they are released by ReleaseByteArrayElements. Is this normal?

[twystd]

In theory, ReleaseByteArrayElements releases the jbytes array and doesn't touch the bytes array but that might not be entirely true if release is being called with discard set true. Also odd that it's only Android 6 .. hmm ...

I'll have to dig into this - thanks for the report and if you have any other stack traces they could be very helpful.

[headuck]

Thanks. The CryptoSignOpen method does set discard to YES when calling release, so that may be the issue?

A few more samples from the app at google play, seems to be at random places when the heap is used with malloc.

When I scroll down the list (sorted by frequency), a few cases are also noted from Android 5.0 / 5.1. There are a few hundred cases from Android 6 last week (which translates to roughly 1% chance per call but I am not yet able to replicate), but no crashes from 7-9.

  #00  pc 00000000000444e4  /system/lib/libc.so (tgkill+12)
   #01  pc 00000000000420f1  /system/lib/libc.so (pthread_kill+32)
   #02  pc 000000000001bb6f  /system/lib/libc.so (raise+10)
   #03  pc 0000000000018d21  /system/lib/libc.so (__libc_android_abort+34)
   #04  pc 00000000000168e0  /system/lib/libc.so (abort+4)
   #05  pc 000000000001a783  /system/lib/libc.so (__libc_fatal+16)
   #06  pc 000000000002e449  /system/lib/libc.so (__bionic_heap_corruption_error+8)
   #07  pc 000000000002f4c9  /system/lib/libc.so (dlmalloc_real+628)
   #08  pc 00000000000484ad  /system/lib/libc++.so (_Znwj+16)
   #09  pc 0000000000316c85  /system/lib/libart.so (_ZN3art14ReferenceTableC1EPKcjj+64)
   #10  pc 000000000025ea01  /system/lib/libart.so (_ZN3art9JNIEnvExt6CreateEPNS_6ThreadEPNS_9JavaVMExtE+72)
   #11  pc 000000000033dc09  /system/lib/libart.so (_ZN3art6Thread18CreateNativeThreadEP7_JNIEnvP8_jobjectjb+252)
   #12  pc 00000000030c61eb  /system/framework/arm/boot.oat

  #00  pc 00000000000444e4  /system/lib/libc.so (tgkill+12)
   #01  pc 00000000000420f1  /system/lib/libc.so (pthread_kill+32)
   #02  pc 000000000001bb6f  /system/lib/libc.so (raise+10)
   #03  pc 0000000000018d21  /system/lib/libc.so (__libc_android_abort+34)
   #04  pc 00000000000168e0  /system/lib/libc.so (abort+4)
   #05  pc 000000000001a783  /system/lib/libc.so (__libc_fatal+16)
   #06  pc 000000000002e449  /system/lib/libc.so (__bionic_heap_corruption_error+8)
   #07  pc 000000000002f4c9  /system/lib/libc.so (dlmalloc_real+628)
   #08  pc 00000000000484ad  /system/lib/libc++.so (_Znwj+16)
   #09  pc 0000000000117ccd  /system/lib/libart.so (_ZNSt3__16vectorIPcNS_9allocatorIS1_EEE21__push_back_slow_pathIRKS1_EEvOT_+96)
   #10  pc 000000000017b9ab  /system/lib/libart.so (_ZN3art2gc9allocator8RosAlloc8BulkFreeEPNS_6ThreadEPPvj+906)
   #11  pc 0000000000195d05  /system/lib/libart.so (_ZN3art2gc9collector9MarkSweep10SweepArrayEPNS0_10accounting11AtomicStackINS_6mirror6ObjectEEEb+816)
   #12  pc 0000000000195765  /system/lib/libart.so (_ZN3art2gc9collector9MarkSweep12ReclaimPhaseEv+228)
   #13  pc 0000000000199e23  /system/lib/libart.so (_ZN3art2gc9collector9MarkSweep9RunPhasesEv+218)
   #14  pc 000000000018fbad  /system/lib/libart.so (_ZN3art2gc9collector16GarbageCollector3RunENS0_7GcCauseEb+276)
   #15  pc 00000000001b2201  /system/lib/libart.so (_ZN3art2gc4Heap22CollectGarbageInternalENS0_9collector6GcTypeENS0_7GcCauseEb+1468)
   #16  pc 00000000001b375b  /system/lib/libart.so (_ZN3art2gc4Heap16ConcurrentGCTask3RunEPNS_6ThreadE+82)
   #17  pc 00000000001cd481  /system/lib/libart.so (_ZN3art2gc13TaskProcessor11RunAllTasksEPNS_6ThreadE+40)
   #18  pc 0000000002b31a2f  /system/framework/arm/boot.oat

 #00  pc 00000000000444e4  /system/lib/libc.so (tgkill+12)
   #01  pc 00000000000420f1  /system/lib/libc.so (pthread_kill+32)
   #02  pc 000000000001bb6f  /system/lib/libc.so (raise+10)
   #03  pc 0000000000018d21  /system/lib/libc.so (__libc_android_abort+34)
   #04  pc 00000000000168e0  /system/lib/libc.so (abort+4)
   #05  pc 000000000001a783  /system/lib/libc.so (__libc_fatal+16)
   #06  pc 000000000002e449  /system/lib/libc.so (__bionic_heap_corruption_error+8)
   #07  pc 000000000002f4c9  /system/lib/libc.so (dlmalloc_real+628)
   #08  pc 0000000000030bd7  /system/lib/libc.so (dlrealloc+130)
   #09  pc 000000000008ca47  /system/lib/libicuuc.so (_ZN6icu_559UVector6414expandCapacityEiR10UErrorCode+90)
   #10  pc 00000000000e141b  /system/lib/libicui18n.so (_ZN6icu_5512RegexCompile8appendOpEi+34)
   #11  pc 00000000000e4df1  /system/lib/libicui18n.so (_ZN6icu_5512RegexCompile14doParseActionsEi+840)
   #12  pc 00000000000e694f  /system/lib/libicui18n.so (_ZN6icu_5512RegexCompile7compileEP5UTextR11UParseErrorR10UErrorCode+242)
   #13  pc 00000000000e6b4f  /system/lib/libicui18n.so 
(_ZN6icu_5512RegexCompile7compileERKNS_13UnicodeStringER11UParseErrorR10UErrorCode+90)
   #14  pc 00000000000f513f  /system/lib/libicui18n.so
   #15  pc 000000000001795d  /system/lib/libjavacore.so
   #16  pc 000000000321e261  /system/framework/arm/boot.oat

  #00  pc 0000000000044338  /system/lib/libc.so (tgkill+12)
   #01  pc 0000000000041f39  /system/lib/libc.so (pthread_kill+32)
   #02  pc 000000000001ba4f  /system/lib/libc.so (raise+10)
   #03  pc 0000000000018bf1  /system/lib/libc.so (__libc_android_abort+34)
   #04  pc 00000000000167b0  /system/lib/libc.so (abort+4)
   #05  pc 000000000001a663  /system/lib/libc.so (__libc_fatal+16)
   #06  pc 000000000002e249  /system/lib/libc.so (__bionic_heap_corruption_error+8)
   #07  pc 000000000002f2e5  /system/lib/libc.so (dlmalloc_real+628)
   #08  pc 000000000000e955  /system/lib/libutils.so (_ZN7android12SharedBuffer5allocEj+28)
   #09  pc 00000000000111a9  /system/lib/libutils.so (_ZN7android10VectorImpl7_shrinkEjj+84)
   #10  pc 0000000000012d75  /system/lib/libutils.so (_ZN7android6Looper9pollInnerEi+80)
   #11  pc 0000000000013007  /system/lib/libutils.so (_ZN7android6Looper8pollOnceEiPiS1_PPv+130)
   #12  pc 0000000000083ea9  /system/lib/libandroid_runtime.so (_ZN7android18NativeMessageQueue8pollOnceEP7_JNIEnvP8_jobjecti+22)
   #13  pc 0000000001f3e575  /system/framework/arm/boot.oat

[twystd]

Hi,

I'm afraid those stack traces are all over the place:

• libart (Android ART library)
• libicuuc (ICU ?)
• libutils

Stack traces with the line /data/app/com.headuck.headuckblocker.dev-1/lib/arm/libtweetnacl.so would be directly attributable to tweetnacl.

I'll dig some more into the tweetnacl trace - I need to bring the project up to date with the new Android build process anyway, but at this stage it looks like there may be something else that is causing multiple problems in your app.

[twystd]

I've been through the code again and it does actually look correct so the problem in your app may lie elsewhere. I'll keep looking and if you can provide any more information that would help - in the meantime you could try a couple of alternatives:

• tweetnacl-java
• libsodium-jni (the underlying crypto is the same)

[headuck]

Thank you. Just to report that I have made a slight modification for tweetnacl-jni.c and now the problem goes away.

Basically, after examining the source of some implementations of ReleaseByteArrayElements in Java VMs (e.g. Kaffe), I suspect that the original code below may be memseting heap memory that has already been freed.

if (bytes) {
	(*env)->ReleaseByteArrayElements(env,jbytes,bytes,discard ? JNI_ABORT : 0);
}
if (copied) {
	memset(bytes,0,N);
}

If this is the case, and it happens that some other threads allocated the just-freed block of memory from the heap before memset, the heap would be corrupted, By its nature this can happen in any thread within the process that touches the heap (as alloc and free would check heap integrity), thus explaining the variety of places reporting heap corruption.

In my modification (headuck@8109d79#diff-ff632c5026b90d3e4749bbe8a95f9d2b) I split the release into two calls in the hope that memset would not be called after free.

This is pretty much all the changes made in the native side since the last version. Now over 10k installs of the new version have been running for a week and there is not yet any heap corruption reports in the new version.

[twystd]

That's a really interesting find! I'll see if I can find some way to positively test but the fact that your problem 'went away' is fairly conclusive.

Thanks very much for digging into this - would you mind submitting a pull request?