Enhance DB recovery logic 40/274440/3
authorTomasz Swierczek <t.swierczek@samsung.com>
Thu, 28 Apr 2022 09:13:42 +0000 (11:13 +0200)
committerTomasz Swierczek <t.swierczek@samsung.com>
Thu, 5 May 2022 10:12:19 +0000 (12:12 +0200)
"If we are wise, let us prepare for the worst."
- George Washington

Previously, the logic of DB recovery was:

1. Remove the "-recovered" file flag, IF it survived reboot (shouldn't)
2. Check DB for corruption
3. IF corruption occured, then:
   a. Replace original DB with fallback made at image creation
   b. Create the "-recovered" file next to DB file that signals rest of the system some apps may be missing

If sudden poweroff happens between 3a and 3b, system will not get informed
about missing app installation data.

This patch changes order of operations 3a and 3b, and also removes
operation number 1. From now on, the system-level scripts responsible
for recovery should remove the flag, when full recovery was complete.

Changing order of 3a with 3b ensures the flag is created when
DB error was found and is not prone to sudden power-off.

The flag is meant to be used for file-existance signalling of the need
to reinstall apps that were not in the backed-up DB. Since its existence
can trigger app installation, which in turn, can launch & use security-manager
(which will also attemt to access the DB), it MUST be ensured that rules-loader
is not running concurrently with any other processes/services that may use security-manager's DB
(the recovery of DB from fallback/backup has to be complete). This is achieved
by systemd's "Before=" service option in rules-loader service file which prohibits
security-manager's socket & service start before rules loader-ends operation.

Change-Id: I472c09d9398f69a97e118b69aad61dc016e3d22d

src/common/include/db-config.h
src/common/privilege_db.cpp
src/server/rules-loader/security-manager-rules-loader.cpp
test/privilege_db_fixture.cpp

index 6d06fb0..4a77266 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2019-2022 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -36,9 +36,8 @@
 #define DB_FALLBACK_PATH DATA_INSTALL_DIR "/" DB_FILE
 
 
-// If database initialization fails, restoration to a fallback snapshot is
-// attempted. If the restoration succeeds, a file flag is created to notify
-// other system components.
+// If database initialization fails, a file flag is created to notify
+// other system components & then, the DB restoration is attempted.
 // For database placed in "$f" the filename is ("$f" DB_RECOVERED_SUFFIX).
 #define DB_RECOVERED_SUFFIX "-recovered"
 #define DB_JOURNAL_SUFFIX "-journal"
index 33ffbc0..1031875 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2014-2022 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -137,8 +137,7 @@ void tryCatchDbInit(F &&f) {
 } //namespace
 
 PrivilegeDb::PrivilegeDb(Offline offline, const std::string &dbPath, const char *okMarkerPath, const char *loaderCmd) {
-    bool didFallback = false;
-    if (!underlying(offline) && !FS::fileExists(okMarkerPath) && !(didFallback = FS::fileExists(dbPath + DB_RECOVERED_SUFFIX)))
+    if (!underlying(offline) && !FS::fileExists(okMarkerPath) && !FS::fileExists(dbPath + DB_RECOVERED_SUFFIX))
         throwDbInitEx("loader failed to initialize db - giving up");
 
     tryCatchDbInit([&]{ mSqlConnection.Connect(dbPath); });
@@ -147,8 +146,6 @@ PrivilegeDb::PrivilegeDb(Offline offline, const std::string &dbPath, const char
     } catch (DB::SqlConnection::Exception::Base &e) {
         if (underlying(offline))
             throwDbInitEx("failed to initialize db in offline mode - giving up");
-        if (didFallback)
-            throwDbInitEx("Database initialization error during query preparation on fallback db - giving up");
         LogError("Database initialization error during query preparation - attempting fallback");
         tryCatchDbInit([&]{
             mSqlConnection.Disconnect();
index 1eeef96..b0dc389 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2018-2022 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -571,7 +571,7 @@ inl void parseOptions(int argc, char *argv[]) {
                 // skip main database bringup and start applying fallback right away
                 //
                 // the option is used by the manager if a fallback has not been applied yet
-                // ("database successfully recovered" marker file does not exist)
+                // ("database recovery attempted" marker file does not exist)
                 // and a schema error has been detected
                 case 'f':
                     // exclusive option - cannot be combined w/ anything else
@@ -1054,7 +1054,7 @@ int main(int argc, char *argv[]) {
 
     // need the following filenames:
     // dbPath : to open the database
-    // dbPath + DB_RECOVERED_SUFFIX : to remove/create the "database successfully recovered" marker file
+    // dbPath + DB_RECOVERED_SUFFIX : to remove/create the "database recovery attempted" marker file
     // dbPath + DB_JOURNAL_SUFFIX : to potentially truncate the journal file when overwriting database with fallback
     //
     // the pkgsInfo tape memory is used to store these names (the tape is otherwise unused during database bringup)
@@ -1068,11 +1068,6 @@ int main(int argc, char *argv[]) {
 
     // pkgsInfo.t = dbPath (\0-unterminated)
     memcpy(pkgsInfo.t, dbPath, dbPathLen);
-    // pkgsInfo.t = dbPath + DB_RECOVERED_SUFFIX (\0-terminated)
-    acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX);
-    // remove the "database successfully recovered" marker file (it's not supposed to survive reboot but is stored on permanent flash)
-    if (unlikely(!unlinkIfExists(pkgsInfo.t)))
-        fail("unlink(db" DB_RECOVERED_SUFFIX ") failed");
 
     parseOptions(argc, argv);
 
@@ -1088,6 +1083,11 @@ int main(int argc, char *argv[]) {
         if (unlikely(0 > creat(dbOkMarker, 0644)))
             fail("creat(dbOkMarker) failed");
     } else {
+        // create the "database recovery attempted" marker file
+        acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX);
+        if (unlikely(creat(pkgsInfo.t, 0644) < 0))
+            fail("creat(.security-manager.db" DB_RECOVERED_SUFFIX ") failed");
+
         // main db initialization failed or not attempted, apply fallback
         toStderr("overwriting db with fallback");
         overwriteDbFileWithFallback(dbPathLen);
@@ -1096,11 +1096,6 @@ int main(int argc, char *argv[]) {
         if (unlikely(!dbUp(CheckFallback::no)))
             fail("fallback db bringup failed");
 
-        // create the "database successfully recovered" marker file
-        acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX);
-        if (unlikely(creat(pkgsInfo.t, 0644) < 0))
-            fail("creat(.security-manager.db" DB_RECOVERED_SUFFIX ") failed");
-
         // try to sync DB_INSTALL_DIR dir
         const int dbdirfd = open(DB_INSTALL_DIR, O_RDONLY);
         if (unlikely(dbdirfd < 0))
index 62557b7..16369e4 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016-2020 Samsung Electronics Co., Ltd. All rights reserved.
+ * Copyright (c) 2016-2022 Samsung Electronics Co., Ltd. All rights reserved.
  *
  * This file is licensed under the terms of MIT License or the Apache License
  * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details.
@@ -69,8 +69,9 @@ void testMarkerFile(const char *f, bool present) {
     }
 }
 void checkMarker(PrivilegeDBFixture::Marker marker) {
-    testMarkerFile(TEST_DB_OK_MARKER, underlying(marker) & underlying(PrivilegeDBFixture::Marker::standard));
-    testMarkerFile(TEST_DB_PATH DB_RECOVERED_SUFFIX, underlying(marker) & underlying(PrivilegeDBFixture::Marker::fallback));
+    const auto broken = marker != PrivilegeDBFixture::Marker::standard;
+    testMarkerFile(TEST_DB_OK_MARKER, !broken);
+    testMarkerFile(TEST_DB_PATH DB_RECOVERED_SUFFIX, broken);
     if (underlying(marker)) {
         struct stat st;
         BOOST_REQUIRE(!lstat(TEST_DB_PATH, &st));