sqlcipher / android-database-sqlcipher

Android SQLite API based on SQLCipher
https://www.zetetic.net/sqlcipher/sqlcipher-for-android/
Other
2.73k stars 564 forks source link

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

Open aneeetha opened 10 months ago

aneeetha commented 10 months ago

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

backtrace:

00 pc 0x0000000000051ba8 /apex/com.android.runtime/lib64/bionic/libc.so (abort+164)

01 pc 0x000000000004171c /apex/com.android.runtime/lib64/bionic/libc.so (scudo::die()+8)

02 pc 0x0000000000041dc8 /apex/com.android.runtime/lib64/bionic/libc.so (scudo::ScopedErrorReport::~ScopedErrorReport()+32)

03 pc 0x0000000000042128 /apex/com.android.runtime/lib64/bionic/libc.so (scudo::reportInvalidChunkState(scudo::AllocatorAction, void*)+116)

04 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)

05 pc 0x00000000001d5e28 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so

06 pc 0x0000000000140660 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so

07 pc 0x00000000000ca640 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so

08 pc 0x00000000000d3f44 /data/app/~~brUaJPOp5HwTzGRwB57bMg==/com.example.redacted-b71M8dXHyQEN-c_xJiL7sA==/split_config.arm64_v8a.apk!libsqlcipher.so (sqlite3_finalize+264)

09 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::InvokeVirtualOrInterfaceWithJValues<art::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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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 support@zetetic.net to coordinate access to a prerelease build of the library for testing in your environment.