[Fix #3831] Apply per-DB instance attach locking (#3862)

osquery/devtools/shell.cpp

@@ -790,6 +790,8 @@ static int shell_exec(
   }
 
   while (zSql[0] && (SQLITE_OK == rc)) {
+    auto lock(dbc->attachLock());
+
     /* A lock for attaching virtual tables, but also the SQL object states. */
     rc = sqlite3_prepare_v2(db, zSql, -1, &pStmt, &zLeftover);
     if (SQLITE_OK != rc) {

osquery/sql/sqlite_util.cpp

@@ -203,7 +203,7 @@ void SQLiteSQLPlugin::detach(const std::string& name) {
   if (!dbc->isPrimary()) {
     return;
   }
-  detachTableInternal(name, dbc->db());
+  detachTableInternal(name, dbc);
 }
 
 SQLiteDBInstance::SQLiteDBInstance(sqlite3*& db, Mutex& mtx)
@@ -251,6 +251,10 @@ bool SQLiteDBInstance::useCache() const {
   return use_cache_;
 }
 
+WriteLock SQLiteDBInstance::attachLock() const {
+  return WriteLock(attach_mutex_);
+}
+
 void SQLiteDBInstance::addAffectedTable(VirtualTableContent* table) {
   // An xFilter/scan was requested for this virtual table.
   affected_tables_.insert(std::make_pair(table->name, table));

osquery/sql/sqlite_util.h

@@ -77,6 +77,9 @@ class SQLiteDBInstance : private boost::noncopyable {
   /// Check if the query requested use of the warm query cache.
   bool useCache() const;
 
+  /// Lock the database for attaching virtual tables.
+  WriteLock attachLock() const;
+
  private:
   /// Handle the primary/forwarding requests for table attribute accesses.
   TableAttributes getAttributes() const;
@@ -102,6 +105,9 @@ class SQLiteDBInstance : private boost::noncopyable {
   /// An attempted unique lock on the manager's primary database access mutex.
   WriteLock lock_;
 
+  /// Attaching can occur async from the registry APIs.
+  mutable Mutex attach_mutex_;
+
   /// Vector of tables that need their constraints cleared after execution.
   std::map<std::string, VirtualTableContent*> affected_tables_;
 

osquery/sql/virtual_table.cpp

@@ -624,7 +624,7 @@ Status attachTableInternal(const std::string& name,
 
   // Note, if the clientData API is used then this will save a registry call
   // within xCreate.
-  RecursiveLock lock(kAttachMutex);
+  auto lock(instance->attachLock());
   int rc = sqlite3_create_module(
       instance->db(), name.c_str(), &module, (void*)&(*instance));
   if (rc == SQLITE_OK || rc == SQLITE_MISUSE) {
@@ -637,10 +637,11 @@ Status attachTableInternal(const std::string& name,
   return Status(rc, getStringForSQLiteReturnCode(rc));
 }
 
-Status detachTableInternal(const std::string& name, sqlite3* db) {
-  RecursiveLock lock(kAttachMutex);
+Status detachTableInternal(const std::string& name,
+                           const SQLiteDBInstanceRef& instance) {
+  auto lock(instance->attachLock());
   auto format = "DROP TABLE IF EXISTS temp." + name;
-  int rc = sqlite3_exec(db, format.c_str(), nullptr, nullptr, 0);
+  int rc = sqlite3_exec(instance->db(), format.c_str(), nullptr, nullptr, 0);
   if (rc != SQLITE_OK) {
     LOG(ERROR) << "Error detaching table: " << name << " (" << rc << ")";
   }
@@ -655,7 +656,7 @@ Status attachFunctionInternal(
   // Hold the manager connection instance again in callbacks.
   auto dbc = SQLiteDBManager::get();
   // Add some shell-specific functions to the instance.
-  RecursiveLock lock(kAttachMutex);
+  auto lock(dbc->attachLock());
   int rc = sqlite3_create_function(
       dbc->db(),
       name.c_str(),

osquery/sql/virtual_table.h

@@ -84,7 +84,8 @@ Status attachTableInternal(const std::string& name,
                            const SQLiteDBInstanceRef& instance);
 
 /// Detach (drop) a table.
-Status detachTableInternal(const std::string& name, sqlite3* db);
+Status detachTableInternal(const std::string& name,
+                           const SQLiteDBInstanceRef& instance);
 
 Status attachFunctionInternal(
     const std::string& name,