Merge changes from topic "refactor-init-servicelist" into main

* changes:
  init: remove interface checks from init
  host_init_verifier: check interface names directly
  init_parser_fuzzer: remove interface checks
This commit is contained in:
Jooyung Han 2024-08-26 07:55:35 +00:00 committed by Gerrit Code Review
commit efbc08e0ce
13 changed files with 46 additions and 144 deletions

View file

@ -38,7 +38,6 @@ init_common_sources = [
"capabilities.cpp",
"epoll.cpp",
"import_parser.cpp",
"interface_utils.cpp",
"interprocess_fifo.cpp",
"keychords.cpp",
"parser.cpp",
@ -88,6 +87,7 @@ init_device_sources = [
init_host_sources = [
"check_builtins.cpp",
"host_import_parser.cpp",
"interface_utils.cpp",
]
soong_config_module_type {
@ -190,7 +190,6 @@ libinit_cc_defaults {
"libext4_utils",
"libfs_mgr",
"libgsi",
"libhidl-gen-utils",
"liblog",
"liblogwrap",
"liblp",

View file

@ -30,7 +30,6 @@ cc_defaults {
shared_libs: [
"libbase",
"libfs_mgr",
"libhidl-gen-utils",
"liblog",
"libprocessgroup",
"libselinux",
@ -49,7 +48,6 @@ cc_fuzz {
srcs: [
"init_parser_fuzzer.cpp",
],
shared_libs: ["libhidlmetadata",],
defaults: [
"libinit_fuzzer_defaults",
],

View file

@ -15,9 +15,7 @@
*/
#include <fuzzer/FuzzedDataProvider.h>
#include <hidl/metadata.h>
#include <import_parser.h>
#include <interface_utils.h>
#include <rlimit_parser.h>
using namespace android;
@ -34,7 +32,6 @@ const std::string kValidPaths[] = {
};
const int32_t kMaxBytes = 256;
const std::string kValidInterfaces = "android.frameworks.vr.composer@2.0::IVrComposerClient";
class InitParserFuzzer {
public:
@ -44,9 +41,6 @@ class InitParserFuzzer {
private:
void InvokeParser();
void InvokeLimitParser();
void InvokeInterfaceUtils();
InterfaceInheritanceHierarchyMap GenerateHierarchyMap();
std::vector<HidlInterfaceMetadata> GenerateInterfaceMetadata();
FuzzedDataProvider fdp_;
};
@ -64,60 +58,6 @@ void InitParserFuzzer::InvokeLimitParser() {
}
}
std::vector<HidlInterfaceMetadata> InitParserFuzzer::GenerateInterfaceMetadata() {
std::vector<HidlInterfaceMetadata> random_interface;
for (size_t idx = 0; idx < fdp_.ConsumeIntegral<size_t>(); ++idx) {
HidlInterfaceMetadata metadata;
metadata.name = fdp_.ConsumeRandomLengthString(kMaxBytes);
for (size_t idx1 = 0; idx1 < fdp_.ConsumeIntegral<size_t>(); ++idx1) {
metadata.inherited.push_back(fdp_.ConsumeRandomLengthString(kMaxBytes));
}
random_interface.push_back(metadata);
}
return random_interface;
}
InterfaceInheritanceHierarchyMap InitParserFuzzer::GenerateHierarchyMap() {
InterfaceInheritanceHierarchyMap result;
std::vector<HidlInterfaceMetadata> random_interface;
if (fdp_.ConsumeBool()) {
random_interface = GenerateInterfaceMetadata();
} else {
random_interface = HidlInterfaceMetadata::all();
}
for (const HidlInterfaceMetadata& iface : random_interface) {
std::set<FQName> inherited_interfaces;
for (const std::string& intf : iface.inherited) {
FQName fqname;
(void)fqname.setTo(intf);
inherited_interfaces.insert(fqname);
}
FQName fqname;
(void)fqname.setTo(iface.name);
result[fqname] = inherited_interfaces;
}
return result;
}
void InitParserFuzzer::InvokeInterfaceUtils() {
InterfaceInheritanceHierarchyMap hierarchy_map = GenerateHierarchyMap();
SetKnownInterfaces(hierarchy_map);
IsKnownInterface(fdp_.ConsumeRandomLengthString(kMaxBytes));
std::set<std::string> interface_set;
for (size_t idx = 0; idx < fdp_.ConsumeIntegral<size_t>(); ++idx) {
auto set_interface_values = fdp_.PickValueInArray<const std::function<void()>>({
[&]() {
interface_set.insert(("aidl/" + fdp_.ConsumeRandomLengthString(kMaxBytes)));
},
[&]() { interface_set.insert(fdp_.ConsumeRandomLengthString(kMaxBytes)); },
[&]() { interface_set.insert(kValidInterfaces); },
});
set_interface_values();
}
CheckInterfaceInheritanceHierarchy(interface_set, hierarchy_map);
}
void InitParserFuzzer::InvokeParser() {
Parser parser;
std::string name = fdp_.ConsumeBool() ? fdp_.ConsumeRandomLengthString(kMaxBytes) : "import";
@ -132,7 +72,6 @@ void InitParserFuzzer::Process() {
while (fdp_.remaining_bytes()) {
auto invoke_parser_fuzzer = fdp_.PickValueInArray<const std::function<void()>>({
[&]() { InvokeParser(); },
[&]() { InvokeInterfaceUtils(); },
[&]() { InvokeLimitParser(); },
});
invoke_parser_fuzzer();

View file

@ -297,9 +297,7 @@ int main(int argc, char** argv) {
ActionManager& am = ActionManager::GetInstance();
ServiceList& sl = ServiceList::GetInstance();
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&sl, GetSubcontext(),
*interface_inheritance_hierarchy_map));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&sl, GetSubcontext()));
parser.AddSectionParser("on", std::make_unique<ActionParser>(&am, GetSubcontext()));
parser.AddSectionParser("import", std::make_unique<HostImportParser>());
@ -317,11 +315,23 @@ int main(int argc, char** argv) {
return EXIT_FAILURE;
}
}
size_t failures = parser.parse_error_count() + am.CheckAllCommands() + sl.CheckAllCommands();
if (failures > 0) {
LOG(ERROR) << "Failed to parse init scripts with " << failures << " error(s).";
return EXIT_FAILURE;
}
for (const auto& service : sl) {
if (const auto& result = CheckInterfaceInheritanceHierarchy(
service->interfaces(), *interface_inheritance_hierarchy_map);
!result.ok()) {
LOG(ERROR) << service->filename() << ": invalid interface in service '"
<< service->name() << "': " << result.error();
return EXIT_FAILURE;
}
}
return EXIT_SUCCESS;
}

View file

@ -268,8 +268,8 @@ void DumpState() {
Parser CreateParser(ActionManager& action_manager, ServiceList& service_list) {
Parser parser;
parser.AddSectionParser("service", std::make_unique<ServiceParser>(
&service_list, GetSubcontext(), std::nullopt));
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, GetSubcontext()));
parser.AddSectionParser("on", std::make_unique<ActionParser>(&action_manager, GetSubcontext()));
parser.AddSectionParser("import", std::make_unique<ImportParser>(&parser));
@ -324,9 +324,7 @@ Parser CreateApexConfigParser(ActionManager& action_manager, ServiceList& servic
}
}
#endif // RECOVERY
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, subcontext,
std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, subcontext));
parser.AddSectionParser("on", std::make_unique<ActionParser>(&action_manager, subcontext));
return parser;

View file

@ -62,8 +62,7 @@ void TestInit(const std::string& init_script_file, const BuiltinFunctionMap& tes
Action::set_function_map(&test_function_map);
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(service_list, nullptr));
parser.AddSectionParser("on", std::make_unique<ActionParser>(action_manager, nullptr));
parser.AddSectionParser("import", std::make_unique<ImportParser>(&parser));
@ -625,8 +624,7 @@ service A something
ServiceList service_list;
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, nullptr));
ASSERT_TRUE(parser.ParseConfig(tf.path));
@ -657,8 +655,7 @@ service A something
ServiceList service_list;
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, nullptr));
ASSERT_TRUE(parser.ParseConfig(tf.path));
ASSERT_EQ(1u, parser.parse_error_count());

View file

@ -39,27 +39,6 @@ std::string FQNamesToString(const std::set<FQName>& fqnames) {
return android::base::Join(fqname_strings, " ");
}
} // namespace
Result<void> CheckInterfaceInheritanceHierarchy(const std::set<std::string>& instances,
const InterfaceInheritanceHierarchyMap& hierarchy) {
std::set<FQName> interface_fqnames;
for (const std::string& instance : instances) {
// There is insufficient build-time information on AIDL interfaces to check them here
// TODO(b/139307527): Rework how services store interfaces to avoid excess string parsing
if (base::Split(instance, "/")[0] == "aidl") {
continue;
}
FqInstance fqinstance;
if (!fqinstance.setTo(instance)) {
return Error() << "Unable to parse interface instance '" << instance << "'";
}
interface_fqnames.insert(fqinstance.getFqName());
}
return CheckInterfaceInheritanceHierarchy(interface_fqnames, hierarchy);
}
Result<void> CheckInterfaceInheritanceHierarchy(const std::set<FQName>& interfaces,
const InterfaceInheritanceHierarchyMap& hierarchy) {
std::ostringstream error_stream;
@ -90,6 +69,27 @@ Result<void> CheckInterfaceInheritanceHierarchy(const std::set<FQName>& interfac
return {};
}
} // namespace
Result<void> CheckInterfaceInheritanceHierarchy(const std::set<std::string>& instances,
const InterfaceInheritanceHierarchyMap& hierarchy) {
std::set<FQName> interface_fqnames;
for (const std::string& instance : instances) {
// There is insufficient build-time information on AIDL interfaces to check them here
// TODO(b/139307527): Rework how services store interfaces to avoid excess string parsing
if (base::Split(instance, "/")[0] == "aidl") {
continue;
}
FqInstance fqinstance;
if (!fqinstance.setTo(instance)) {
return Error() << "Unable to parse interface instance '" << instance << "'";
}
interface_fqnames.insert(fqinstance.getFqName());
}
return CheckInterfaceInheritanceHierarchy(interface_fqnames, hierarchy);
}
std::optional<std::set<FQName>> known_interfaces;
void SetKnownInterfaces(const InterfaceInheritanceHierarchyMap& hierarchy) {

View file

@ -34,8 +34,6 @@ using InterfaceInheritanceHierarchyMap = std::map<android::FQName, std::set<andr
// interface set. Uses the provided hierarchy data.
Result<void> CheckInterfaceInheritanceHierarchy(const std::set<std::string>& instances,
const InterfaceInheritanceHierarchyMap& hierarchy);
Result<void> CheckInterfaceInheritanceHierarchy(const std::set<android::FQName>& interfaces,
const InterfaceInheritanceHierarchyMap& hierarchy);
// Saves the set of known interfaces using the provided HIDL interface
// inheritance hierarchy.

View file

@ -103,8 +103,7 @@ service $name /system/bin/yes
"$selabel", GetSecurityContext(), false);
ServiceList& service_list = ServiceList::GetInstance();
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, nullptr));
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);

View file

@ -27,7 +27,6 @@
#include <android-base/parseint.h>
#include <android-base/properties.h>
#include <android-base/strings.h>
#include <hidl-util/FQName.h>
#include <processgroup/processgroup.h>
#include <system/thread_defs.h>
@ -201,24 +200,6 @@ Result<void> ServiceParser::ParsePriority(std::vector<std::string>&& args) {
Result<void> ServiceParser::ParseInterface(std::vector<std::string>&& args) {
const std::string& interface_name = args[1];
const std::string& instance_name = args[2];
// AIDL services don't use fully qualified names and instead just use "interface aidl <name>"
if (interface_name != "aidl") {
FQName fq_name;
if (!FQName::parse(interface_name, &fq_name)) {
return Error() << "Invalid fully-qualified name for interface '" << interface_name
<< "'";
}
if (!fq_name.isFullyQualified()) {
return Error() << "Interface name not fully-qualified '" << interface_name << "'";
}
if (fq_name.isValidValueName()) {
return Error() << "Interface name must not be a value name '" << interface_name << "'";
}
}
const std::string fullname = interface_name + "/" + instance_name;
for (const auto& svc : *service_list_) {
@ -702,14 +683,6 @@ Result<void> ServiceParser::EndSection() {
}
}
if (interface_inheritance_hierarchy_) {
if (const auto& check_hierarchy_result = CheckInterfaceInheritanceHierarchy(
service_->interfaces(), *interface_inheritance_hierarchy_);
!check_hierarchy_result.ok()) {
return Error() << check_hierarchy_result.error();
}
}
if (SelinuxGetVendorAndroidVersion() >= __ANDROID_API_R__) {
if ((service_->flags() & SVC_CRITICAL) != 0 && (service_->flags() & SVC_ONESHOT) != 0) {
return Error() << "service '" << service_->name()

View file

@ -18,7 +18,6 @@
#include <vector>
#include "interface_utils.h"
#include "parser.h"
#include "service.h"
#include "service_list.h"
@ -29,13 +28,8 @@ namespace init {
class ServiceParser : public SectionParser {
public:
ServiceParser(
ServiceList* service_list, Subcontext* subcontext,
const std::optional<InterfaceInheritanceHierarchyMap>& interface_inheritance_hierarchy)
: service_list_(service_list),
subcontext_(subcontext),
interface_inheritance_hierarchy_(interface_inheritance_hierarchy),
service_(nullptr) {}
ServiceParser(ServiceList* service_list, Subcontext* subcontext)
: service_list_(service_list), subcontext_(subcontext), service_(nullptr) {}
Result<void> ParseSection(std::vector<std::string>&& args, const std::string& filename,
int line) override;
Result<void> ParseLineSection(std::vector<std::string>&& args, int line) override;
@ -88,7 +82,6 @@ class ServiceParser : public SectionParser {
ServiceList* service_list_;
Subcontext* subcontext_;
std::optional<InterfaceInheritanceHierarchyMap> interface_inheritance_hierarchy_;
std::unique_ptr<Service> service_;
std::string filename_;
};

View file

@ -253,8 +253,7 @@ service $name /system/bin/yes
"$selabel", GetSecurityContext(), false);
ServiceList& service_list = ServiceList::GetInstance();
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, nullptr));
TemporaryFile tf;
ASSERT_GE(tf.fd, 0);

View file

@ -30,8 +30,7 @@ namespace init {
android::base::Result<ServiceInterfacesMap> GetOnDeviceServiceInterfacesMap() {
ServiceList& service_list = ServiceList::GetInstance();
Parser parser;
parser.AddSectionParser("service",
std::make_unique<ServiceParser>(&service_list, nullptr, std::nullopt));
parser.AddSectionParser("service", std::make_unique<ServiceParser>(&service_list, nullptr));
for (const auto& location : {
"/init.rc",
"/system/etc/init",