From 254b9046f70b324cca75857b7bddd12908ade81b Mon Sep 17 00:00:00 2001 From: Neil Fuller Date: Fri, 15 May 2020 10:37:56 +0000 Subject: [PATCH] Revert "libsnapshot_fuzzer: Add tests" This reverts commit 51bfe08d84a547160cfcc743c22075f5aca9d71f. Reason for revert: Investigating possible connection to http://b/156689792 Change-Id: Idd779815940e3835bc0b86103ef016141d48ce7f --- fs_mgr/TEST_MAPPING | 3 - fs_mgr/libsnapshot/Android.bp | 20 +-- fs_mgr/libsnapshot/fuzz_utils.h | 120 +++++++---------- fs_mgr/libsnapshot/snapshot_fuzz.cpp | 189 +++++---------------------- 4 files changed, 84 insertions(+), 248 deletions(-) diff --git a/fs_mgr/TEST_MAPPING b/fs_mgr/TEST_MAPPING index 6cd043013..676f446e7 100644 --- a/fs_mgr/TEST_MAPPING +++ b/fs_mgr/TEST_MAPPING @@ -14,9 +14,6 @@ }, { "name": "vts_libsnapshot_test" - }, - { - "name": "libsnapshot_fuzzer_test" } ] } diff --git a/fs_mgr/libsnapshot/Android.bp b/fs_mgr/libsnapshot/Android.bp index 2783e4de4..c1911022f 100644 --- a/fs_mgr/libsnapshot/Android.bp +++ b/fs_mgr/libsnapshot/Android.bp @@ -246,8 +246,8 @@ cc_test { gtest: false, } -cc_defaults { - name: "libsnapshot_fuzzer_defaults", +cc_fuzz { + name: "libsnapshot_fuzzer", // TODO(b/154633114): make host supported. // host_supported: true, @@ -289,11 +289,6 @@ cc_defaults { canonical_path_from_root: false, local_include_dirs: ["."], }, -} - -cc_fuzz { - name: "libsnapshot_fuzzer", - defaults: ["libsnapshot_fuzzer_defaults"], corpus: ["corpus/*"], fuzz_config: { cc: ["android-virtual-ab+bugs@google.com"], @@ -303,14 +298,3 @@ cc_fuzz { fuzz_on_haiku_device: true, }, } - -cc_test { - name: "libsnapshot_fuzzer_test", - defaults: ["libsnapshot_fuzzer_defaults"], - data: ["corpus/*"], - test_suites: [ - "device-tests", - ], - auto_gen_config: true, - require_root: true, -} diff --git a/fs_mgr/libsnapshot/fuzz_utils.h b/fs_mgr/libsnapshot/fuzz_utils.h index 20b13b2fa..4dc6cdc0b 100644 --- a/fs_mgr/libsnapshot/fuzz_utils.h +++ b/fs_mgr/libsnapshot/fuzz_utils.h @@ -68,25 +68,17 @@ int CheckConsistency() { return 0; } -// Get the field descriptor for the oneof field in the action message. If no oneof field is set, -// return nullptr. template -const google::protobuf::FieldDescriptor* GetValueFieldDescriptor( - const typename Action::Proto& action_proto) { +void ExecuteActionProto(typename Action::Class* module, + const typename Action::Proto& action_proto) { static auto* action_value_desc = GetProtoValueDescriptor(Action::Proto::GetDescriptor()); auto* action_refl = Action::Proto::GetReflection(); if (!action_refl->HasOneof(action_proto, action_value_desc)) { - return nullptr; + return; } - return action_refl->GetOneofFieldDescriptor(action_proto, action_value_desc); -} -template -void ExecuteActionProto(typename Action::ClassType* module, - const typename Action::Proto& action_proto) { - const auto* field_desc = GetValueFieldDescriptor(action_proto); - if (field_desc == nullptr) return; + const auto* field_desc = action_refl->GetOneofFieldDescriptor(action_proto, action_value_desc); auto number = field_desc->number(); const auto& map = *Action::GetFunctionMap(); auto it = map.find(number); @@ -97,7 +89,7 @@ void ExecuteActionProto(typename Action::ClassType* module, template void ExecuteAllActionProtos( - typename Action::ClassType* module, + typename Action::Class* module, const google::protobuf::RepeatedPtrField& action_protos) { for (const auto& proto : action_protos) { ExecuteActionProto(module, proto); @@ -142,57 +134,53 @@ FUZZ_DEFINE_PRIMITIVE_GETTER(float, GetFloat); // ActionPerformer extracts arguments from the protobuf message, and then call FuzzFunction // with these arguments. template -struct ActionPerformerImpl; // undefined +struct ActionPerfomer; // undefined template -struct ActionPerformerImpl< +struct ActionPerfomer< FuzzFunction, void(const MessageProto&), typename std::enable_if_t>> { - static typename FuzzFunction::ReturnType Invoke( - typename FuzzFunction::ClassType* module, const google::protobuf::Message& action_proto, - const google::protobuf::FieldDescriptor* field_desc) { + static void Invoke(typename FuzzFunction::Class* module, + const google::protobuf::Message& action_proto, + const google::protobuf::FieldDescriptor* field_desc) { const MessageProto& arg = CheckedCast>( action_proto.GetReflection()->GetMessage(action_proto, field_desc)); - return FuzzFunction::ImplBody(module, arg); + FuzzFunction::ImplBody(module, arg); } }; template -struct ActionPerformerImpl>> { - static typename FuzzFunction::ReturnType Invoke( - typename FuzzFunction::ClassType* module, const google::protobuf::Message& action_proto, - const google::protobuf::FieldDescriptor* field_desc) { +struct ActionPerfomer>> { + static void Invoke(typename FuzzFunction::Class* module, + const google::protobuf::Message& action_proto, + const google::protobuf::FieldDescriptor* field_desc) { Primitive arg = std::invoke(PrimitiveGetter::fp, action_proto.GetReflection(), action_proto, field_desc); - return FuzzFunction::ImplBody(module, arg); + FuzzFunction::ImplBody(module, arg); } }; template -struct ActionPerformerImpl { - static typename FuzzFunction::ReturnType Invoke(typename FuzzFunction::ClassType* module, - const google::protobuf::Message&, - const google::protobuf::FieldDescriptor*) { - return FuzzFunction::ImplBody(module); +struct ActionPerfomer { + static void Invoke(typename FuzzFunction::Class* module, const google::protobuf::Message&, + const google::protobuf::FieldDescriptor*) { + FuzzFunction::ImplBody(module); } }; template -struct ActionPerformerImpl { - static typename FuzzFunction::ReturnType Invoke( - typename FuzzFunction::ClassType* module, const google::protobuf::Message& action_proto, - const google::protobuf::FieldDescriptor* field_desc) { +struct ActionPerfomer { + static void Invoke(typename FuzzFunction::Class* module, + const google::protobuf::Message& action_proto, + const google::protobuf::FieldDescriptor* field_desc) { std::string scratch; const std::string& arg = action_proto.GetReflection()->GetStringReference( action_proto, field_desc, &scratch); - return FuzzFunction::ImplBody(module, arg); + FuzzFunction::ImplBody(module, arg); } }; -template -struct ActionPerformer : ActionPerformerImpl {}; - } // namespace android::fuzz // Fuzz existing C++ class, ClassType, with a collection of functions under the name Action. @@ -209,11 +197,11 @@ struct ActionPerformer : ActionPerformerImpl; \ static FunctionMap* GetFunctionMap() { \ static Action::FunctionMap map; \ @@ -237,33 +225,29 @@ struct ActionPerformer : ActionPerformerImplDoAwesomeFoo(arg); // } // The name DoFoo is the camel case name of the action in protobuf definition of FooActionProto. -#define FUZZ_FUNCTION(Action, FunctionName, Return, ModuleArg, ...) \ - class FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName) { \ - public: \ - using ActionType = Action; \ - using ClassType = Action::ClassType; \ - using ReturnType = Return; \ - using Signature = void(__VA_ARGS__); \ - static constexpr const char name[] = #FunctionName; \ - static constexpr const auto tag = \ - Action::Proto::ValueCase::FUZZ_FUNCTION_TAG_NAME(FunctionName); \ - static ReturnType ImplBody(ModuleArg, ##__VA_ARGS__); \ - \ - private: \ - static bool registered_; \ - }; \ - auto FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName)::registered_ = ([] { \ - auto tag = FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName)::tag; \ - auto func = &::android::fuzz::ActionPerformer::Invoke; \ - Action::GetFunctionMap()->CheckEmplace(tag, func); \ - return true; \ - })(); \ - Return FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName)::ImplBody(ModuleArg, ##__VA_ARGS__) +#define FUZZ_FUNCTION(Action, FunctionName, module, ...) \ + class FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName) { \ + public: \ + using Class = Action::Class; \ + static void ImplBody(Action::Class*, ##__VA_ARGS__); \ + \ + private: \ + static bool registered_; \ + }; \ + auto FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName)::registered_ = ([] { \ + auto tag = Action::Proto::ValueCase::FUZZ_FUNCTION_TAG_NAME(FunctionName); \ + auto func = \ + &::android::fuzz::ActionPerfomer::Invoke; \ + Action::GetFunctionMap()->CheckEmplace(tag, func); \ + return true; \ + })(); \ + void FUZZ_FUNCTION_CLASS_NAME(Action, FunctionName)::ImplBody(Action::Class* module, \ + ##__VA_ARGS__) // Implement a simple action by linking it to the function with the same name. Example: // message FooActionProto { @@ -277,9 +261,5 @@ struct ActionPerformer : ActionPerformerImpl().FunctionName()), \ - Action::ClassType* module) { \ - return module->FunctionName(); \ - } +#define FUZZ_SIMPLE_FUNCTION(Action, FunctionName) \ + FUZZ_FUNCTION(Action, FunctionName, module) { (void)module->FunctionName(); } diff --git a/fs_mgr/libsnapshot/snapshot_fuzz.cpp b/fs_mgr/libsnapshot/snapshot_fuzz.cpp index 5b145c31f..1e90ace6f 100644 --- a/fs_mgr/libsnapshot/snapshot_fuzz.cpp +++ b/fs_mgr/libsnapshot/snapshot_fuzz.cpp @@ -21,21 +21,14 @@ #include #include -#include -#include -#include #include #include #include "fuzz_utils.h" #include "snapshot_fuzz_utils.h" -using android::base::Error; -using android::base::GetBoolProperty; using android::base::LogId; using android::base::LogSeverity; -using android::base::ReadFileToString; -using android::base::Result; using android::base::SetLogger; using android::base::StderrLogger; using android::base::StdioLogger; @@ -44,8 +37,6 @@ using android::fuzz::CheckedCast; using android::snapshot::SnapshotFuzzData; using android::snapshot::SnapshotFuzzEnv; using chromeos_update_engine::DeltaArchiveManifest; -using google::protobuf::FieldDescriptor; -using google::protobuf::Message; using google::protobuf::RepeatedPtrField; // Avoid linking to libgsi since it needs disk I/O. @@ -83,49 +74,48 @@ FUZZ_SIMPLE_FUNCTION(SnapshotManagerAction, RecoveryCreateSnapshotDevices); FUZZ_SIMPLE_FUNCTION(SnapshotManagerAction, EnsureMetadataMounted); FUZZ_SIMPLE_FUNCTION(SnapshotManagerAction, GetSnapshotMergeStatsInstance); -#define SNAPSHOT_FUZZ_FUNCTION(FunctionName, ReturnType, ...) \ - FUZZ_FUNCTION(SnapshotManagerAction, FunctionName, ReturnType, ISnapshotManager* snapshot, \ - ##__VA_ARGS__) +#define SNAPSHOT_FUZZ_FUNCTION(FunctionName, ...) \ + FUZZ_FUNCTION(SnapshotManagerAction, FunctionName, snapshot, ##__VA_ARGS__) -SNAPSHOT_FUZZ_FUNCTION(FinishedSnapshotWrites, bool, bool wipe) { - return snapshot->FinishedSnapshotWrites(wipe); +SNAPSHOT_FUZZ_FUNCTION(FinishedSnapshotWrites, bool wipe) { + (void)snapshot->FinishedSnapshotWrites(wipe); } -SNAPSHOT_FUZZ_FUNCTION(ProcessUpdateState, bool, const ProcessUpdateStateArgs& args) { +SNAPSHOT_FUZZ_FUNCTION(ProcessUpdateState, const ProcessUpdateStateArgs& args) { std::function before_cancel; if (args.has_before_cancel()) { before_cancel = [&]() { return args.fail_before_cancel(); }; } - return snapshot->ProcessUpdateState({}, before_cancel); + (void)snapshot->ProcessUpdateState({}, before_cancel); } -SNAPSHOT_FUZZ_FUNCTION(GetUpdateState, UpdateState, bool has_progress_arg) { +SNAPSHOT_FUZZ_FUNCTION(GetUpdateState, bool has_progress_arg) { double progress; - return snapshot->GetUpdateState(has_progress_arg ? &progress : nullptr); + (void)snapshot->GetUpdateState(has_progress_arg ? &progress : nullptr); } -SNAPSHOT_FUZZ_FUNCTION(HandleImminentDataWipe, bool, bool has_callback) { +SNAPSHOT_FUZZ_FUNCTION(HandleImminentDataWipe, bool has_callback) { std::function callback; if (has_callback) { callback = []() {}; } - return snapshot->HandleImminentDataWipe(callback); + (void)snapshot->HandleImminentDataWipe(callback); } -SNAPSHOT_FUZZ_FUNCTION(Dump, bool) { +SNAPSHOT_FUZZ_FUNCTION(Dump) { std::stringstream ss; - return snapshot->Dump(ss); + (void)snapshot->Dump(ss); } -SNAPSHOT_FUZZ_FUNCTION(CreateUpdateSnapshots, bool, const DeltaArchiveManifest& manifest) { - return snapshot->CreateUpdateSnapshots(manifest); +SNAPSHOT_FUZZ_FUNCTION(CreateUpdateSnapshots, const DeltaArchiveManifest& manifest) { + (void)snapshot->CreateUpdateSnapshots(manifest); } -SNAPSHOT_FUZZ_FUNCTION(UnmapUpdateSnapshot, bool, const std::string& name) { - return snapshot->UnmapUpdateSnapshot(name); +SNAPSHOT_FUZZ_FUNCTION(UnmapUpdateSnapshot, const std::string& name) { + (void)snapshot->UnmapUpdateSnapshot(name); } -SNAPSHOT_FUZZ_FUNCTION(CreateLogicalAndSnapshotPartitions, bool, +SNAPSHOT_FUZZ_FUNCTION(CreateLogicalAndSnapshotPartitions, const CreateLogicalAndSnapshotPartitionsArgs& args) { const std::string* super; if (args.use_correct_super()) { @@ -133,21 +123,20 @@ SNAPSHOT_FUZZ_FUNCTION(CreateLogicalAndSnapshotPartitions, bool, } else { super = &args.super(); } - return snapshot->CreateLogicalAndSnapshotPartitions( + (void)snapshot->CreateLogicalAndSnapshotPartitions( *super, std::chrono::milliseconds(args.timeout_millis())); } -SNAPSHOT_FUZZ_FUNCTION(RecoveryCreateSnapshotDevicesWithMetadata, CreateResult, +SNAPSHOT_FUZZ_FUNCTION(RecoveryCreateSnapshotDevicesWithMetadata, const RecoveryCreateSnapshotDevicesArgs& args) { std::unique_ptr device; if (args.has_metadata_device_object()) { device = std::make_unique(args.metadata_mounted()); } - return snapshot->RecoveryCreateSnapshotDevices(device); + (void)snapshot->RecoveryCreateSnapshotDevices(device); } -SNAPSHOT_FUZZ_FUNCTION(MapUpdateSnapshot, bool, - const CreateLogicalPartitionParamsProto& params_proto) { +SNAPSHOT_FUZZ_FUNCTION(MapUpdateSnapshot, const CreateLogicalPartitionParamsProto& params_proto) { auto partition_opener = std::make_unique(GetSnapshotFuzzEnv()->super()); CreateLogicalPartitionParams params; if (params_proto.use_correct_super()) { @@ -164,10 +153,10 @@ SNAPSHOT_FUZZ_FUNCTION(MapUpdateSnapshot, bool, params.device_name = params_proto.device_name(); params.partition_opener = partition_opener.get(); std::string path; - return snapshot->MapUpdateSnapshot(params, &path); + (void)snapshot->MapUpdateSnapshot(params, &path); } -SNAPSHOT_FUZZ_FUNCTION(SwitchSlot, void) { +SNAPSHOT_FUZZ_FUNCTION(SwitchSlot) { (void)snapshot; CHECK(current_module != nullptr); CHECK(current_module->device_info != nullptr); @@ -205,8 +194,7 @@ void FatalOnlyLogger(LogId logid, LogSeverity severity, const char* tag, const c } // Stop logging (except fatal messages) after global initialization. This is only done once. int StopLoggingAfterGlobalInit() { - (void)GetSnapshotFuzzEnv(); - [[maybe_unused]] static protobuf_mutator::protobuf::LogSilencer log_silencer; + [[maybe_unused]] static protobuf_mutator::protobuf::LogSilencer log_silincer; SetLogger(&FatalOnlyLogger); return 0; } @@ -214,10 +202,15 @@ int StopLoggingAfterGlobalInit() { SnapshotFuzzEnv* GetSnapshotFuzzEnv() { [[maybe_unused]] static auto allow_logging = AllowLoggingDuringGlobalInit(); static SnapshotFuzzEnv env; + [[maybe_unused]] static auto stop_logging = StopLoggingAfterGlobalInit(); return &env; } -SnapshotTestModule SetUpTest(const SnapshotFuzzData& snapshot_fuzz_data) { +} // namespace android::snapshot + +DEFINE_PROTO_FUZZER(const SnapshotFuzzData& snapshot_fuzz_data) { + using namespace android::snapshot; + current_data = &snapshot_fuzz_data; auto env = GetSnapshotFuzzEnv(); @@ -226,127 +219,9 @@ SnapshotTestModule SetUpTest(const SnapshotFuzzData& snapshot_fuzz_data) { auto test_module = env->CheckCreateSnapshotManager(snapshot_fuzz_data); current_module = &test_module; CHECK(test_module.snapshot); - return test_module; -} -void TearDownTest() { + SnapshotManagerAction::ExecuteAll(test_module.snapshot.get(), snapshot_fuzz_data.actions()); + current_module = nullptr; current_data = nullptr; } - -} // namespace android::snapshot - -DEFINE_PROTO_FUZZER(const SnapshotFuzzData& snapshot_fuzz_data) { - using namespace android::snapshot; - - [[maybe_unused]] static auto stop_logging = StopLoggingAfterGlobalInit(); - auto test_module = SetUpTest(snapshot_fuzz_data); - SnapshotManagerAction::ExecuteAll(test_module.snapshot.get(), snapshot_fuzz_data.actions()); - TearDownTest(); -} - -namespace android::snapshot { - -// Work-around to cast a 'void' value to Result. -template -struct GoodResult { - template - static Result Cast(F&& f) { - return f(); - } -}; - -template <> -struct GoodResult { - template - static Result Cast(F&& f) { - f(); - return {}; - } -}; - -class LibsnapshotFuzzerTest : public ::testing::Test { - protected: - static void SetUpTestCase() { - // Do initialization once. - (void)GetSnapshotFuzzEnv(); - } - void SetUp() override { - bool is_virtual_ab = GetBoolProperty("ro.virtual_ab.enabled", false); - if (!is_virtual_ab) GTEST_SKIP() << "Test only runs on Virtual A/B devices."; - } - void SetUpFuzzData(const std::string& fn) { - auto path = android::base::GetExecutableDirectory() + "/corpus/"s + fn; - std::string proto_text; - ASSERT_TRUE(ReadFileToString(path, &proto_text)); - snapshot_fuzz_data_ = std::make_unique(); - ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString(proto_text, - snapshot_fuzz_data_.get())); - test_module_ = android::snapshot::SetUpTest(*snapshot_fuzz_data_); - } - void TearDown() override { android::snapshot::TearDownTest(); } - template - Result Execute(int action_index) { - if (action_index >= snapshot_fuzz_data_->actions_size()) { - return Error() << "Index " << action_index << " is out of bounds (" - << snapshot_fuzz_data_->actions_size() << " actions in corpus"; - } - const auto& action_proto = snapshot_fuzz_data_->actions(action_index); - const auto* field_desc = - android::fuzz::GetValueFieldDescriptor( - action_proto); - if (field_desc == nullptr) { - return Error() << "Action at index " << action_index << " has no value defined."; - } - if (FuzzFunction::tag != field_desc->number()) { - return Error() << "Action at index " << action_index << " is expected to be " - << FuzzFunction::name << ", but it is " << field_desc->name() - << " in corpus."; - } - return GoodResult::Cast([&]() { - return android::fuzz::ActionPerformer::Invoke(test_module_.snapshot.get(), - action_proto, field_desc); - }); - } - - std::unique_ptr snapshot_fuzz_data_; - SnapshotTestModule test_module_; -}; - -#define SNAPSHOT_FUZZ_FN_NAME(name) FUZZ_FUNCTION_CLASS_NAME(SnapshotManagerAction, name) - -MATCHER_P(ResultIs, expected, "") { - if (!arg.ok()) { - *result_listener << arg.error(); - return false; - } - *result_listener << "expected: " << expected; - return arg.value() == expected; -} - -#define ASSERT_RESULT_TRUE(actual) ASSERT_THAT(actual, ResultIs(true)) - -// Check that launch_device.txt is executed correctly. -TEST_F(LibsnapshotFuzzerTest, LaunchDevice) { - SetUpFuzzData("launch_device.txt"); - - int i = 0; - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)) << "sys_b"; - ASSERT_RESULT_TRUE(Execute(i++)) << "vnd_b"; - ASSERT_RESULT_TRUE(Execute(i++)) << "prd_b"; - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)) << "sys_b"; - ASSERT_RESULT_TRUE(Execute(i++)) << "vnd_b"; - ASSERT_RESULT_TRUE(Execute(i++)) << "prd_b"; - ASSERT_RESULT_OK(Execute(i++)); - ASSERT_EQ("_b", test_module_.device_info->GetSlotSuffix()); - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_RESULT_TRUE(Execute(i++)); - ASSERT_EQ(i, snapshot_fuzz_data_->actions_size()) << "Not all actions are executed."; -} - -} // namespace android::snapshot