Merge "trusty: Apploader fuzzer"
diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp
index c1a59d8..ca68212 100644
--- a/debuggerd/libdebuggerd/tombstone.cpp
+++ b/debuggerd/libdebuggerd/tombstone.cpp
@@ -592,7 +592,6 @@
}
ProcessInfo process_info;
- unique_fd attr_fd(open("/proc/self/attr/current", O_RDONLY | O_CLOEXEC));
process_info.abort_msg_address = abort_msg_address;
engrave_tombstone(unique_fd(dup(tombstone_fd)), unique_fd(dup(proto_fd)), &unwinder, threads, tid,
process_info, nullptr, nullptr);
diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp
index 8ac3361..785a8e0 100644
--- a/fs_mgr/fs_mgr_fstab.cpp
+++ b/fs_mgr/fs_mgr_fstab.cpp
@@ -412,7 +412,8 @@
if (!fs_mgr_get_boot_config(prop, &suffix)) continue;
- for (const char* prefix : {"/odm/etc/fstab.", "/vendor/etc/fstab.", "/fstab."}) {
+ for (const char* prefix :
+ {"/odm/etc/fstab.", "/vendor/etc/fstab.", "/fstab.", "/first_stage_ramdisk/fstab."}) {
std::string fstab_path = prefix + suffix;
if (access(fstab_path.c_str(), F_OK) == 0) {
return fstab_path;
diff --git a/fs_mgr/libfiemap/image_manager.cpp b/fs_mgr/libfiemap/image_manager.cpp
index 841f215..44f659b 100644
--- a/fs_mgr/libfiemap/image_manager.cpp
+++ b/fs_mgr/libfiemap/image_manager.cpp
@@ -16,6 +16,8 @@
#include <libfiemap/image_manager.h>
+#include <optional>
+
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/properties.h>
@@ -574,7 +576,7 @@
return false;
}
auto& dm = DeviceMapper::Instance();
- LoopControl loop;
+ std::optional<LoopControl> loop;
std::string status;
auto status_file = GetStatusFilePath(name);
@@ -598,9 +600,14 @@
return false;
}
} else if (pieces[0] == "loop") {
+ // Lazily connect to loop-control to avoid spurious errors in recovery.
+ if (!loop.has_value()) {
+ loop.emplace();
+ }
+
// Failure to remove a loop device is not fatal, since we can still
// remove the backing file if we want.
- loop.Detach(pieces[1]);
+ loop->Detach(pieces[1]);
} else {
LOG(ERROR) << "Unknown status: " << pieces[0];
}
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
index 0d90f6c..a79a86d 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot.h
@@ -694,8 +694,8 @@
// Call ProcessUpdateState and handle states with special rules before data wipe. Specifically,
// if |allow_forward_merge| and allow-forward-merge indicator exists, initiate merge if
// necessary.
- bool ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
- const std::function<bool()>& callback);
+ UpdateState ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
+ const std::function<bool()>& callback);
// Return device string of a mapped image, or if it is not available, the mapped image path.
bool GetMappedImageDeviceStringOrPath(const std::string& device_name,
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index eb3a501..cc2599d 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -894,6 +894,8 @@
const std::function<bool()>& before_cancel) {
while (true) {
UpdateState state = CheckMergeState(before_cancel);
+ LOG(INFO) << "ProcessUpdateState handling state: " << state;
+
if (state == UpdateState::MergeFailed) {
AcknowledgeMergeFailure();
}
@@ -920,13 +922,15 @@
}
UpdateState state = CheckMergeState(lock.get(), before_cancel);
+ LOG(INFO) << "CheckMergeState for snapshots returned: " << state;
+
if (state == UpdateState::MergeCompleted) {
// Do this inside the same lock. Failures get acknowledged without the
// lock, because flock() might have failed.
AcknowledgeMergeSuccess(lock.get());
} else if (state == UpdateState::Cancelled) {
- if (!RemoveAllUpdateState(lock.get(), before_cancel)) {
- return ReadSnapshotUpdateStatus(lock.get()).state();
+ if (!device_->IsRecovery() && !RemoveAllUpdateState(lock.get(), before_cancel)) {
+ LOG(ERROR) << "Failed to remove all update state after acknowleding cancelled update.";
}
}
return state;
@@ -968,13 +972,23 @@
return UpdateState::MergeFailed;
}
+ auto other_suffix = device_->GetOtherSlotSuffix();
+
bool cancelled = false;
bool failed = false;
bool merging = false;
bool needs_reboot = false;
bool wrong_phase = false;
for (const auto& snapshot : snapshots) {
+ if (android::base::EndsWith(snapshot, other_suffix)) {
+ // This will have triggered an error message in InitiateMerge already.
+ LOG(INFO) << "Skipping merge validation of unexpected snapshot: " << snapshot;
+ continue;
+ }
+
UpdateState snapshot_state = CheckTargetMergeState(lock, snapshot, update_status);
+ LOG(INFO) << "CheckTargetMergeState for " << snapshot << " returned: " << snapshot_state;
+
switch (snapshot_state) {
case UpdateState::MergeFailed:
failed = true;
@@ -1173,7 +1187,7 @@
// indicator that cleanup is needed on reboot. If a factory data reset
// was requested, it doesn't matter, everything will get wiped anyway.
// To make testing easier we consider a /data wipe as cleaned up.
- if (device_->IsRecovery() && !in_factory_data_reset_) {
+ if (device_->IsRecovery()) {
WriteUpdateState(lock, UpdateState::MergeCompleted);
return;
}
@@ -1692,6 +1706,7 @@
for (const auto& snapshot : snapshots) {
DmTargetSnapshot::Status current_status;
+ if (!IsSnapshotDevice(snapshot)) continue;
if (!QuerySnapshotStatus(snapshot, nullptr, ¤t_status)) continue;
fake_snapshots_status.sectors_allocated += current_status.sectors_allocated;
@@ -3212,10 +3227,11 @@
};
in_factory_data_reset_ = true;
- bool ok = ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
+ UpdateState state =
+ ProcessUpdateStateOnDataWipe(true /* allow_forward_merge */, process_callback);
in_factory_data_reset_ = false;
- if (!ok) {
+ if (state == UpdateState::MergeFailed) {
return false;
}
@@ -3223,6 +3239,16 @@
if (!UnmapAllPartitionsInRecovery()) {
LOG(ERROR) << "Unable to unmap all partitions; fastboot may fail to flash.";
}
+
+ if (state != UpdateState::None) {
+ auto lock = LockExclusive();
+ if (!lock) return false;
+
+ // Zap the update state so the bootloader doesn't think we're still
+ // merging. It's okay if this fails, it's informative only at this
+ // point.
+ WriteUpdateState(lock.get(), UpdateState::None);
+ }
return true;
}
@@ -3257,15 +3283,15 @@
return true;
}
-bool SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
- const std::function<bool()>& callback) {
+UpdateState SnapshotManager::ProcessUpdateStateOnDataWipe(bool allow_forward_merge,
+ const std::function<bool()>& callback) {
auto slot_number = SlotNumberForSlotSuffix(device_->GetSlotSuffix());
UpdateState state = ProcessUpdateState(callback);
LOG(INFO) << "Update state in recovery: " << state;
switch (state) {
case UpdateState::MergeFailed:
LOG(ERROR) << "Unrecoverable merge failure detected.";
- return false;
+ return state;
case UpdateState::Unverified: {
// If an OTA was just applied but has not yet started merging:
//
@@ -3285,8 +3311,12 @@
if (allow_forward_merge &&
access(GetForwardMergeIndicatorPath().c_str(), F_OK) == 0) {
LOG(INFO) << "Forward merge allowed, initiating merge now.";
- return InitiateMerge() &&
- ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
+
+ if (!InitiateMerge()) {
+ LOG(ERROR) << "Failed to initiate merge on data wipe.";
+ return UpdateState::MergeFailed;
+ }
+ return ProcessUpdateStateOnDataWipe(false /* allow_forward_merge */, callback);
}
LOG(ERROR) << "Reverting to old slot since update will be deleted.";
@@ -3304,7 +3334,7 @@
default:
break;
}
- return true;
+ return state;
}
bool SnapshotManager::EnsureNoOverflowSnapshot(LockedFile* lock) {
diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index d57aa6c..bde4cca 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -636,8 +636,8 @@
// Because the status is Merging, we must call ProcessUpdateState, which should
// detect a cancelled update.
- ASSERT_EQ(sm->ProcessUpdateState(), UpdateState::Cancelled);
- ASSERT_EQ(sm->GetUpdateState(), UpdateState::None);
+ ASSERT_EQ(init->ProcessUpdateState(), UpdateState::Cancelled);
+ ASSERT_EQ(init->GetUpdateState(), UpdateState::None);
}
TEST_F(SnapshotTest, UpdateBootControlHal) {
@@ -1767,7 +1767,7 @@
ASSERT_TRUE(new_sm->HandleImminentDataWipe());
// Manually mount metadata so that we can call GetUpdateState() below.
MountMetadata();
- EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::Unverified);
+ EXPECT_EQ(new_sm->GetUpdateState(), UpdateState::None);
EXPECT_TRUE(test_device->IsSlotUnbootable(1));
EXPECT_FALSE(test_device->IsSlotUnbootable(0));
}
@@ -2105,8 +2105,12 @@
// There should be no snapshot to merge.
auto new_sm = SnapshotManager::New(new TestDeviceInfo(fake_super, flashed_slot_suffix));
- // update_enigne calls ProcessUpdateState first -- should see Cancelled.
- ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
+ if (flashed_slot == 0 && after_merge) {
+ ASSERT_EQ(UpdateState::MergeCompleted, new_sm->ProcessUpdateState());
+ } else {
+ // update_engine calls ProcessUpdateState first -- should see Cancelled.
+ ASSERT_EQ(UpdateState::Cancelled, new_sm->ProcessUpdateState());
+ }
// Next OTA calls CancelUpdate no matter what.
ASSERT_TRUE(new_sm->CancelUpdate());
diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp
index de72f23..a11bb28 100644
--- a/init/first_stage_mount.cpp
+++ b/init/first_stage_mount.cpp
@@ -44,6 +44,7 @@
#include "block_dev_initializer.h"
#include "devices.h"
+#include "result.h"
#include "snapuserd_transition.h"
#include "switch_root.h"
#include "uevent.h"
@@ -51,6 +52,7 @@
#include "util.h"
using android::base::ReadFileToString;
+using android::base::Result;
using android::base::Split;
using android::base::StringPrintf;
using android::base::Timer;
@@ -81,7 +83,7 @@
// The factory method to create either FirstStageMountVBootV1 or FirstStageMountVBootV2
// based on device tree configurations.
- static std::unique_ptr<FirstStageMount> Create();
+ static Result<std::unique_ptr<FirstStageMount>> Create();
bool DoCreateDevices(); // Creates devices and logical partitions from storage devices
bool DoFirstStageMount(); // Mounts fstab entries read from device tree.
bool InitDevices();
@@ -160,7 +162,7 @@
return is_android_dt_value_expected("vbmeta/compatible", "android,vbmeta");
}
-static Fstab ReadFirstStageFstab() {
+static Result<Fstab> ReadFirstStageFstab() {
Fstab fstab;
if (!ReadFstabFromDt(&fstab)) {
if (ReadDefaultFstab(&fstab)) {
@@ -170,7 +172,7 @@
}),
fstab.end());
} else {
- LOG(INFO) << "Failed to fstab for first stage mount";
+ return Error() << "failed to read default fstab for first stage mount";
}
}
return fstab;
@@ -236,12 +238,16 @@
super_partition_name_ = fs_mgr_get_super_partition_name();
}
-std::unique_ptr<FirstStageMount> FirstStageMount::Create() {
+Result<std::unique_ptr<FirstStageMount>> FirstStageMount::Create() {
auto fstab = ReadFirstStageFstab();
- if (IsDtVbmetaCompatible(fstab)) {
- return std::make_unique<FirstStageMountVBootV2>(std::move(fstab));
+ if (!fstab.ok()) {
+ return fstab.error();
+ }
+
+ if (IsDtVbmetaCompatible(*fstab)) {
+ return std::make_unique<FirstStageMountVBootV2>(std::move(*fstab));
} else {
- return std::make_unique<FirstStageMountVBootV1>(std::move(fstab));
+ return std::make_unique<FirstStageMountVBootV1>(std::move(*fstab));
}
}
@@ -836,12 +842,12 @@
// ----------------
// Creates devices and logical partitions from storage devices
bool DoCreateDevices() {
- std::unique_ptr<FirstStageMount> handle = FirstStageMount::Create();
- if (!handle) {
- LOG(ERROR) << "Failed to create FirstStageMount";
+ auto fsm = FirstStageMount::Create();
+ if (!fsm.ok()) {
+ LOG(ERROR) << "Failed to create FirstStageMount: " << fsm.error();
return false;
}
- return handle->DoCreateDevices();
+ return (*fsm)->DoCreateDevices();
}
// Mounts partitions specified by fstab in device tree.
@@ -852,17 +858,17 @@
return true;
}
- std::unique_ptr<FirstStageMount> handle = FirstStageMount::Create();
- if (!handle) {
- LOG(ERROR) << "Failed to create FirstStageMount";
+ auto fsm = FirstStageMount::Create();
+ if (!fsm.ok()) {
+ LOG(ERROR) << "Failed to create FirstStageMount " << fsm.error();
return false;
}
if (create_devices) {
- if (!handle->DoCreateDevices()) return false;
+ if (!(*fsm)->DoCreateDevices()) return false;
}
- return handle->DoFirstStageMount();
+ return (*fsm)->DoFirstStageMount();
}
void SetInitAvbVersionInRecovery() {
@@ -872,8 +878,12 @@
}
auto fstab = ReadFirstStageFstab();
+ if (!fstab.ok()) {
+ LOG(ERROR) << fstab.error();
+ return;
+ }
- if (!IsDtVbmetaCompatible(fstab)) {
+ if (!IsDtVbmetaCompatible(*fstab)) {
LOG(INFO) << "Skipped setting INIT_AVB_VERSION (not vbmeta compatible)";
return;
}
@@ -883,7 +893,7 @@
// We only set INIT_AVB_VERSION when the AVB verification succeeds, i.e., the
// Open() function returns a valid handle.
// We don't need to mount partitions here in recovery mode.
- FirstStageMountVBootV2 avb_first_mount(std::move(fstab));
+ FirstStageMountVBootV2 avb_first_mount(std::move(*fstab));
if (!avb_first_mount.InitDevices()) {
LOG(ERROR) << "Failed to init devices for INIT_AVB_VERSION";
return;
diff --git a/libprocessgroup/setup/cgroup_map_write.cpp b/libprocessgroup/setup/cgroup_map_write.cpp
index 753fd2d..aa41acb 100644
--- a/libprocessgroup/setup/cgroup_map_write.cpp
+++ b/libprocessgroup/setup/cgroup_map_write.cpp
@@ -183,10 +183,12 @@
return false;
}
- Json::Reader reader;
+ Json::CharReaderBuilder builder;
+ std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
Json::Value root;
- if (!reader.parse(json_doc, root)) {
- LOG(ERROR) << "Failed to parse cgroups description: " << reader.getFormattedErrorMessages();
+ std::string errorMessage;
+ if (!reader->parse(&*json_doc.begin(), &*json_doc.end(), &root, &errorMessage)) {
+ LOG(ERROR) << "Failed to parse cgroups description: " << errorMessage;
return false;
}
diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp
index 8d4ce25..f13a681 100644
--- a/libprocessgroup/task_profiles.cpp
+++ b/libprocessgroup/task_profiles.cpp
@@ -425,10 +425,12 @@
return false;
}
- Json::Reader reader;
+ Json::CharReaderBuilder builder;
+ std::unique_ptr<Json::CharReader> reader(builder.newCharReader());
Json::Value root;
- if (!reader.parse(json_doc, root)) {
- LOG(ERROR) << "Failed to parse task profiles: " << reader.getFormattedErrorMessages();
+ std::string errorMessage;
+ if (!reader->parse(&*json_doc.begin(), &*json_doc.end(), &root, &errorMessage)) {
+ LOG(ERROR) << "Failed to parse task profiles: " << errorMessage;
return false;
}