Thread synchronisation issue in SQLiteCompiledSQL.releaseReference() which causes a double free in SQLiteCompiledSQL native_finalize

[aneeetha]

---

pid: 0, tid: 8948 >>> com.example.redacted <<<

backtrace:
#00 pc 0x0000000000051ba8 /apex/com.android.runtime/lib64/bionic/libc.so (abort+164)
#1 pc 0x000000000004171c /apex/com.android.runtime/lib64/bionic/libc.so (scudo::die()+8)
#2 pc 0x0000000000041dc8 /apex/com.android.runtime/lib64/bionic/libc.so (scudo::ScopedErrorReport::~ScopedErrorReport()+32)
#3 pc 0x0000000000042128 /apex/com.android.runtime/lib64/bionic/libc.so (scudo::reportInvalidChunkState(scudo::AllocatorAction, void*)+116)
#4 pc 0x0000000000043928 /apex/com.android.runtime/lib64/bionic/libc.so (scudo::Allocator<scudo::AndroidConfig, &(scudo_malloc_postinit)>::deallocate(void*, scudo::Chunk::Origin, unsigned long, unsigned long)+308)
#5 pc 0x00000000001d5e28 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so
#6 pc 0x0000000000140660 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so
#7 pc 0x00000000000ca640 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so
#8 pc 0x00000000000d3f44 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so (sqlite3_finalize+264)
#9 pc 0x000000000023961c /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so
#10 pc 0x000000000016c1a4 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/oat/arm64/base.odex (art_jni_trampoline+116)
#11 pc 0x00000000005ef9ec /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/oat/arm64/base.odex (net.sqlcipher.database.SQLiteCompiledSql.releaseSqlStatement+348)
#12 pc 0x00000000005ef618 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/oat/arm64/base.odex (net.sqlcipher.database.SQLiteCompiledSql.finalize+376)
#13 pc 0x00000000006382a8 /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (java.lang.Daemons$FinalizerDaemon.doFinalize+104)
#14 pc 0x0000000000638598 /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (java.lang.Daemons$FinalizerDaemon.runInternal+584)
#15 pc 0x0000000000604c14 /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (java.lang.Daemons$Daemon.run+180)
#16 pc 0x00000000003fe6b0 /data/misc/apexdata/com.android.art/dalvik-cache/arm64/boot.oat (java.lang.Thread.run+80)
#17 pc 0x000000000045836c /apex/com.android.art/lib64/libart.so (art_quick_invoke_stub+556)
#18 pc 0x00000000004841e4 /apex/com.android.art/lib64/libart.so (art::ArtMethod::Invoke(art::Thread*, unsigned int*, unsigned int, art::JValue*, char const*)+156)
#19 pc 0x0000000000483eb0 /apex/com.android.art/lib64/libart.so (art::JValue art::InvokeVirtualOrInterfaceWithJValuesart::ArtMethod*(art::ScopedObjectAccessAlreadyRunnable const&, _jobject*, art::ArtMethod*, jvalue const*)+400)
#20 pc 0x00000000005cc668 /apex/com.android.art/lib64/libart.so (art::Thread::CreateCallback(void*)+1680)
#21 pc 0x00000000000b6668 /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+208)
#22 pc 0x00000000000532cc /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64)

In our application we often perform db operations. These operations are performed in a multi threaded environment, that we even perform db operations in one thread and close the cursor in another thread. We are facing an issue when SQLiteCompiledSql.releaseSqlStatement is called. This issue occurs when the SQL cache is completely full and the newly created SQLITE Statements are not stored in the cache. In this case when the SQLiteCursor.close() is invoked it will call SQLProgram.close() which again calls releaseRerference() which again invokes onAllReferencesReleased() which invokes releaseCompiledSQLIfnotinCache()

In releaseCompiledSQLIfnotinCache(), it invokes SQLiteCompiledSQL.releaseReference() where the releaseReference() method is not synchronised properly. so this might invoke native_finalize() double times which causes the error.

As per our analysis, we can see the problem if the SQLiteDatabase.close() is called in one thread and at the same time if any opened cursor is closed from an another thread there is a possibility that these two threads will reach to SqliteCompiledSQL.releaseReference() at the same time which can cause a double free in C++ layer.

When checking SQLiteDatabase.close() we can see that it invokes SQliteDatabase.closeClosable() and in that method all the SQLprograms stored in map are iterated and try to release the reference.

In short the issue here is

On two separate threads in a cache full scenario if one thread is trying to close the database which will iterate over all the SQLPrograms and invoke SQLiteCompiledSQL.releaseReference() and on the same time if another thread tries to close the cursor related to this SQLiteCompiledSQL there will be a scenario where two different threads can access the same SQLiteCompiledSQL.releaseReference() which will invoke a double free in C++ layer.

SQLCipher version: 4.5.3

Have added backtrace for reference.

[developernotes]

Hello @aneeetha,

My apologizes for the delay in response. We have pushed up a change we suspect should prevent the double free scenario. It would be best to avoid invoking operations with the same statement across multiple threads without coordination. Would you build the try out the latest in master and let us know your results?

[muthus55]

@developernotes Thanks for your response.

Can you confirm us one thing ?

Right now you have made the releaseStatement() as synchronized. Does this cause any Thread deadlock issues since this same releaseStatement() will be called from finalize() method ? as some dalvik VM and Android Runtime runs garbage collection by pausing the thread execution and then performing GC on the same paused thread ?

Also can you tag this issue as a new version and provide as a release ? Building SQLCipher from scratch we have to map all the required dependencies and NDK with the exact version.

[developernotes]

Hi @muthus55,

Apologizes for the delay in response. I am not aware that a deadlock should occur in that circumstance. We do not have a reproduction case within the test suite that represents the issue being reported above. With the change that was made, the test suite passed successfully.

In terms of tagging the release, we only do that with public releases which are coordinated with SQLCipher core releases typically. Please reach out to us directly at [email protected] to coordinate access to a prerelease build of the library for testing in your environment.