update_engine: Invalidate firmware from Omaha
Currently if Omaha triggers an update invalidation, the update engine
only invalidates kernel + root updates, but not firmware.
Add the firmware invalidation to Omaha-triggered invalidations, too.
BUG=b:275530794
TEST=manually updated firmware and issued invalidation from Omaha
TEST=cros_run_unit_tests --board=<BOARD> --packages update_engine
Change-Id: I9d0001333b3c0b65d0e57a908ab34b364f3f22d3
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/4622720
Reviewed-by: Hung-Te Lin <[email protected]>
Commit-Queue: Artyom Chen <[email protected]>
Tested-by: Artyom Chen <[email protected]>
Reviewed-by: Jae Hoon Kim <[email protected]>
diff --git a/BUILD.gn b/BUILD.gn
index 2dbedac..8ebc94e 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -262,6 +262,7 @@
all_dependent_pkg_deps = [
"dbus-1",
"expat",
+ "libcrossystem",
"libcurl",
"libdebugd-client",
"libmetrics",
@@ -596,6 +597,7 @@
pkg_deps = [
"libbrillo-test",
"libchrome-test",
+ "libcrossystem-test",
"libdebugd-client-test",
"libpower_manager-client-test",
"libsession_manager-client-test",
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 4fe9c5f..44bcccb 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -274,6 +274,8 @@
return rootfs_verification_enabled_;
}
+ bool ResetFWTryNextSlot() override { return true; };
+
private:
bool is_official_build_{true};
bool is_normal_boot_mode_{true};
diff --git a/common/hardware_interface.h b/common/hardware_interface.h
index 7422220..03355b5 100644
--- a/common/hardware_interface.h
+++ b/common/hardware_interface.h
@@ -187,6 +187,10 @@
// Returns true if rootfs verification is enabled.
// e.g. "dm_verity.dev_wait" is set to 1 in the kernel commandline.
virtual bool IsRootfsVerificationEnabled() const = 0;
+
+ // Resets a RW firmware partition slot to try on next boot to a current slot.
+ // Returns false on failure, true on success.
+ virtual bool ResetFWTryNextSlot() = 0;
};
} // namespace chromeos_update_engine
diff --git a/cros/hardware_chromeos.cc b/cros/hardware_chromeos.cc
index 2b900a3..c95cd09 100644
--- a/cros/hardware_chromeos.cc
+++ b/cros/hardware_chromeos.cc
@@ -27,6 +27,7 @@
#include <brillo/blkdev_utils/lvm.h>
#include <brillo/key_value_store.h>
#include <debugd/dbus-constants.h>
+#include <libcrossystem/crossystem.h>
#include <vboot/crossystem.h>
extern "C" {
@@ -106,6 +107,22 @@
constexpr char kEnrollmentRecoveryRequired[] = "EnrollmentRecoveryRequired";
constexpr char kConsumerSegment[] = "IsConsumerSegment";
+
+// Firmware slot to try next (A or B).
+constexpr char kFWTryNextFlag[] = "fw_try_next";
+
+// Current main firmware.
+constexpr char kMainFWActFlag[] = "mainfw_act";
+
+// Firmware boot result this boot.
+constexpr char kFWResultFlag[] = "fw_result";
+
+// Number of times to try to boot `kFWTryNextFlag` slot.
+constexpr char kFWTryCountFlag[] = "fw_try_count";
+
+// Firmware partition slots.
+constexpr char kFWSlotA[] = "A";
+constexpr char kFWSlotB[] = "B";
} // namespace
namespace chromeos_update_engine {
@@ -128,6 +145,7 @@
LoadConfig("" /* root_prefix */, IsNormalBootMode());
debugd_proxy_.reset(
new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus()));
+ crossystem_.reset(new crossystem::Crossystem());
}
bool HardwareChromeOS::IsOfficialBuild() const {
@@ -556,4 +574,69 @@
return kernel_cmd_line.find("dm_verity.dev_wait=1") != std::string::npos;
}
+bool HardwareChromeOS::ResetFWTryNextSlot() {
+ const std::optional<std::string> main_fw_act = GetMainFWAct();
+ const int fw_try_count = 0;
+
+ if (!main_fw_act) {
+ return false;
+ }
+
+ return SetFWTryNextSlot(*main_fw_act) && SetFWResultSuccessful() &&
+ SetFWTryCount(fw_try_count);
+}
+
+bool HardwareChromeOS::SetFWTryNextSlot(base::StringPiece target_slot) {
+ DCHECK(crossystem_);
+
+ if (target_slot != kFWSlotA && target_slot != kFWSlotB) {
+ LOG(ERROR) << "Invalid target_slot " << target_slot;
+ return false;
+ }
+
+ if (!crossystem_->VbSetSystemPropertyString(kFWTryNextFlag,
+ target_slot.data())) {
+ LOG(ERROR) << "Unable to set " << kFWTryNextFlag << " to "
+ << target_slot.data();
+ return false;
+ }
+
+ return true;
+}
+
+std::optional<std::string> HardwareChromeOS::GetMainFWAct() const {
+ DCHECK(crossystem_);
+
+ const std::optional<std::string> main_fw_act =
+ crossystem_->VbGetSystemPropertyString(kMainFWActFlag);
+ if (!main_fw_act) {
+ LOG(ERROR) << "Unable to get a current FW slot from " << kMainFWActFlag;
+ return std::nullopt;
+ }
+
+ return *main_fw_act;
+}
+
+bool HardwareChromeOS::SetFWResultSuccessful() {
+ DCHECK(crossystem_);
+
+ if (!crossystem_->VbSetSystemPropertyString(kFWResultFlag, "success")) {
+ LOG(ERROR) << "Unable to set " << kFWResultFlag << " to success";
+ return false;
+ }
+
+ return true;
+}
+
+bool HardwareChromeOS::SetFWTryCount(int count) {
+ DCHECK(crossystem_);
+
+ if (!crossystem_->VbSetSystemPropertyInt(kFWTryCountFlag, count)) {
+ LOG(ERROR) << "Unable to set " << kFWTryCountFlag << " to " << count;
+ return false;
+ }
+
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/cros/hardware_chromeos.h b/cros/hardware_chromeos.h
index 471b34c..3b1c7be 100644
--- a/cros/hardware_chromeos.h
+++ b/cros/hardware_chromeos.h
@@ -24,6 +24,7 @@
#include <base/time/time.h>
#include <debugd/dbus-proxies.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
+#include <libcrossystem/crossystem.h>
#include "update_engine/common/error_code.h"
#include "update_engine/common/hardware_interface.h"
@@ -86,11 +87,37 @@
non_volatile_path_ = path;
}
+ bool ResetFWTryNextSlot() override;
+
private:
friend class HardwareChromeOSTest;
FRIEND_TEST(HardwareChromeOSTest, GeneratePowerwashCommandCheck);
FRIEND_TEST(HardwareChromeOSTest,
GeneratePowerwashCommandWithRollbackDataCheck);
+ FRIEND_TEST(HardwareChromeOSFWTest,
+ ResetsFWTryNextSlotProperlyIfValidMainFwAct);
+ FRIEND_TEST(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfInvalidMainFwAct);
+ FRIEND_TEST(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfMissingMainFwAct);
+ FRIEND_TEST(HardwareChromeOSTest,
+ ResetFWTryNextSlotFailsIfSettingResultFlagFails);
+ FRIEND_TEST(HardwareChromeOSTest,
+ ResetFWTryNextSlotFailsIfSettingTryCountFails);
+
+ // Returns a currently active firmware slot.
+ // `kFWSlotA` or `kFWSlotB` most of the time, though can be "recovery" or
+ // "error".
+ std::optional<std::string> GetMainFWAct() const;
+ // Sets a RW firmware partition slot to try on next boot to
+ // |target_slot|.
+ // Expects only `kFWSlotA` or `kFWSlotB` as |target_slot|.
+ // Returns false on failure.
+ bool SetFWTryNextSlot(base::StringPiece target_slot);
+ // Marks current RW firmware boot result as success.
+ // Returns false on failure.
+ bool SetFWResultSuccessful();
+ // Sets a number of times to try a next boot RW partition slot to |count|.
+ // Returns false on failure.
+ bool SetFWTryCount(int count);
// Load the update manager config flags (is_oobe_enabled flag) from the
// appropriate location based on whether we are in a normal mode boot (as
@@ -107,6 +134,7 @@
base::FilePath non_volatile_path_;
std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_;
+ std::unique_ptr<crossystem::Crossystem> crossystem_;
};
} // namespace chromeos_update_engine
diff --git a/cros/hardware_chromeos_unittest.cc b/cros/hardware_chromeos_unittest.cc
index 391d683..fc769f8 100644
--- a/cros/hardware_chromeos_unittest.cc
+++ b/cros/hardware_chromeos_unittest.cc
@@ -17,12 +17,15 @@
#include "update_engine/cros/hardware_chromeos.h"
#include <memory>
+#include <utility>
#include <base/files/file_util.h>
#include <base/files/scoped_temp_dir.h>
#include <base/json/json_string_value_serializer.h>
#include <brillo/file_utils.h>
#include <gtest/gtest.h>
+#include <libcrossystem/crossystem.h>
+#include <libcrossystem/crossystem_fake.h>
#include "update_engine/common/constants.h"
#include "update_engine/common/fake_hardware.h"
@@ -75,6 +78,12 @@
void SetUp() override {
ASSERT_TRUE(root_dir_.CreateUniqueTempDir());
FakeSystemState::CreateInstance();
+
+ auto fake_crossystem_impl =
+ std::make_unique<crossystem::fake::CrossystemFake>();
+ auto crossystem = std::make_unique<crossystem::Crossystem>(
+ std::move(fake_crossystem_impl));
+ hardware_.crossystem_ = std::move(crossystem);
}
void WriteStatefulConfig(const string& config) {
@@ -378,4 +387,84 @@
EXPECT_FALSE(hardware_.IsConsumerSegmentSet(root.get()));
}
+struct FWTestCase {
+ std::string mainfw_act;
+ std::string fw_try_next;
+};
+
+class HardwareChromeOSFWTest : public HardwareChromeOSTest,
+ public testing::WithParamInterface<FWTestCase> {
+};
+
+INSTANTIATE_TEST_SUITE_P(
+ /* no prefix */,
+ HardwareChromeOSFWTest,
+ testing::Values(FWTestCase{.mainfw_act = "A", .fw_try_next = "B"},
+ FWTestCase{.mainfw_act = "B", .fw_try_next = "A"}));
+
+TEST_P(HardwareChromeOSFWTest, ResetsFWTryNextSlotProperlyIfValidMainFwAct) {
+ auto fw_test_case = GetParam();
+
+ hardware_.crossystem_->VbSetSystemPropertyInt("fw_try_count", 5);
+ hardware_.crossystem_->VbSetSystemPropertyString("mainfw_act",
+ fw_test_case.mainfw_act);
+ hardware_.crossystem_->VbSetSystemPropertyString("fw_try_next",
+ fw_test_case.fw_try_next);
+ hardware_.crossystem_->VbSetSystemPropertyString("fw_result", "unknown");
+
+ bool result = hardware_.ResetFWTryNextSlot();
+
+ ASSERT_TRUE(result);
+ EXPECT_EQ(hardware_.crossystem_->VbGetSystemPropertyString("mainfw_act"),
+ fw_test_case.mainfw_act);
+ EXPECT_EQ(hardware_.crossystem_->VbGetSystemPropertyString("fw_try_next"),
+ hardware_.crossystem_->VbGetSystemPropertyString("mainfw_act"));
+ EXPECT_EQ(hardware_.crossystem_->VbGetSystemPropertyInt("fw_try_count"), 0);
+ EXPECT_EQ(hardware_.crossystem_->VbGetSystemPropertyString("fw_result"),
+ "success");
+}
+
+TEST_F(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfInvalidMainFwAct) {
+ hardware_.crossystem_->VbSetSystemPropertyString("mainfw_act", "recovery");
+
+ bool result = hardware_.ResetFWTryNextSlot();
+
+ ASSERT_FALSE(result);
+}
+
+TEST_F(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfMissingMainFwAct) {
+ auto fake_crossystem = std::make_unique<crossystem::fake::CrossystemFake>();
+ fake_crossystem->UnsetSystemPropertyValue("mainfw_act");
+ hardware_.crossystem_ =
+ std::make_unique<crossystem::Crossystem>(std::move(fake_crossystem));
+
+ bool result = hardware_.ResetFWTryNextSlot();
+
+ ASSERT_FALSE(result);
+}
+
+TEST_F(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfSettingResultFlagFails) {
+ auto fake_crossystem = std::make_unique<crossystem::fake::CrossystemFake>();
+ fake_crossystem->SetSystemPropertyReadOnlyStatus("fw_result", true);
+ hardware_.crossystem_ =
+ std::make_unique<crossystem::Crossystem>(std::move(fake_crossystem));
+ hardware_.crossystem_->VbSetSystemPropertyString("mainfw_act", "A");
+
+ bool result = hardware_.ResetFWTryNextSlot();
+
+ ASSERT_FALSE(result);
+}
+
+TEST_F(HardwareChromeOSTest, ResetFWTryNextSlotFailsIfSettingTryCountFails) {
+ auto fake_crossystem = std::make_unique<crossystem::fake::CrossystemFake>();
+ fake_crossystem->SetSystemPropertyReadOnlyStatus("fw_try_count", true);
+ hardware_.crossystem_ =
+ std::make_unique<crossystem::Crossystem>(std::move(fake_crossystem));
+ hardware_.crossystem_->VbSetSystemPropertyString("mainfw_act", "A");
+
+ bool result = hardware_.ResetFWTryNextSlot();
+
+ ASSERT_FALSE(result);
+}
+
} // namespace chromeos_update_engine
diff --git a/cros/update_attempter.cc b/cros/update_attempter.cc
index c022a90..d049827 100644
--- a/cros/update_attempter.cc
+++ b/cros/update_attempter.cc
@@ -1728,6 +1728,11 @@
<< "Could not delete update completed markers. Continuing anyway.";
success = false;
}
+ LOG(INFO) << "Invalidating firmware update.";
+ if (!SystemState::Get()->hardware()->ResetFWTryNextSlot()) {
+ LOG(WARNING) << "Could not reset firmware slot. Continuing anyway.";
+ success = false;
+ }
SystemState::Get()->metrics_reporter()->ReportInvalidatedUpdate(success);
}
