Skip to content

Commit c4cb341

Browse files
committed
Fix TOCTOU in exqlite_close: move conn->db NULL check inside lock
Two concurrent close() calls both passed the pre-lock `conn->db == NULL` check. One would close the db and set conn->db = NULL inside the lock; the second would then call sqlite3_get_autocommit(NULL) → SIGSEGV. Move the NULL check inside the critical section so only one thread can ever observe and act on a non-NULL conn->db. Fixes: "concurrent double close does not segfault" regression test.
1 parent 8741c06 commit c4cb341

1 file changed

Lines changed: 17 additions & 7 deletions

File tree

c_src/sqlite3_nif.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -364,16 +364,20 @@ exqlite_close(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
364364
return make_error_tuple(env, am_invalid_connection);
365365
}
366366

367-
// DB is already closed, nothing to do here
368-
if (conn->db == NULL) {
369-
return am_ok;
370-
}
371-
372367
// close connection in critical section to avoid race-condition
373368
// cases. Cases such as query timeout and connection pooling
374369
// attempting to close the connection
375370
connection_acquire_lock(conn);
376371

372+
// DB is already closed, nothing to do here.
373+
// Check must be inside the lock: two concurrent closes both pass
374+
// a pre-lock NULL check, and the second would call
375+
// sqlite3_get_autocommit(NULL) → segfault.
376+
if (conn->db == NULL) {
377+
connection_release_lock(conn);
378+
return am_ok;
379+
}
380+
377381
int autocommit = sqlite3_get_autocommit(conn->db);
378382
if (autocommit == 0) {
379383
rc = sqlite3_exec(conn->db, "ROLLBACK;", NULL, NULL, NULL);
@@ -920,6 +924,10 @@ exqlite_last_insert_rowid(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
920924
}
921925

922926
connection_acquire_lock(conn);
927+
if (conn->db == NULL) {
928+
connection_release_lock(conn);
929+
return make_error_tuple(env, am_connection_closed);
930+
}
923931
sqlite3_int64 last_rowid = sqlite3_last_insert_rowid(conn->db);
924932
connection_release_lock(conn);
925933
return make_ok_tuple(env, enif_make_int64(env, last_rowid));
@@ -947,11 +955,13 @@ exqlite_transaction_status(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
947955
// and then re-opens a new connection. There is a condition where by
948956
// the connection's database is not set but the calling elixir / erlang
949957
// pass an incomplete reference.
958+
// Check must be inside the lock: a concurrent close() can set conn->db = NULL
959+
// between the pre-lock check and the sqlite3_get_autocommit() call → segfault.
960+
connection_acquire_lock(conn);
950961
if (!conn->db) {
962+
connection_release_lock(conn);
951963
return make_ok_tuple(env, am_error);
952964
}
953-
954-
connection_acquire_lock(conn);
955965
int autocommit = sqlite3_get_autocommit(conn->db);
956966
connection_release_lock(conn);
957967

0 commit comments

Comments
 (0)