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
diff --git a/init/Android.bp b/init/Android.bp
index ed1f148..8f1ca3d 100644
--- a/init/Android.bp
+++ b/init/Android.bp
@@ -38,7 +38,6 @@
"capabilities.cpp",
"epoll.cpp",
"import_parser.cpp",
- "interface_utils.cpp",
"interprocess_fifo.cpp",
"keychords.cpp",
"parser.cpp",
@@ -88,6 +87,7 @@
init_host_sources = [
"check_builtins.cpp",
"host_import_parser.cpp",
+ "interface_utils.cpp",
]
soong_config_module_type {
@@ -190,7 +190,6 @@
"libext4_utils",
"libfs_mgr",
"libgsi",
- "libhidl-gen-utils",
"liblog",
"liblogwrap",
"liblp",
diff --git a/init/fuzzer/Android.bp b/init/fuzzer/Android.bp
index 5823932..8cfd597 100644
--- a/init/fuzzer/Android.bp
+++ b/init/fuzzer/Android.bp
@@ -30,7 +30,6 @@
shared_libs: [
"libbase",
"libfs_mgr",
- "libhidl-gen-utils",
"liblog",
"libprocessgroup",
"libselinux",
@@ -49,7 +48,6 @@
srcs: [
"init_parser_fuzzer.cpp",
],
- shared_libs: ["libhidlmetadata",],
defaults: [
"libinit_fuzzer_defaults",
],
diff --git a/init/fuzzer/init_parser_fuzzer.cpp b/init/fuzzer/init_parser_fuzzer.cpp
index dc76465..21b04f4 100644
--- a/init/fuzzer/init_parser_fuzzer.cpp
+++ b/init/fuzzer/init_parser_fuzzer.cpp
@@ -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 int32_t kMaxBytes = 256;
-const std::string kValidInterfaces = "android.frameworks.vr.composer@2.0::IVrComposerClient";
class InitParserFuzzer {
public:
@@ -44,9 +41,6 @@
private:
void InvokeParser();
void InvokeLimitParser();
- void InvokeInterfaceUtils();
- InterfaceInheritanceHierarchyMap GenerateHierarchyMap();
- std::vector<HidlInterfaceMetadata> GenerateInterfaceMetadata();
FuzzedDataProvider fdp_;
};
@@ -64,60 +58,6 @@
}
}
-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 @@
while (fdp_.remaining_bytes()) {
auto invoke_parser_fuzzer = fdp_.PickValueInArray<const std::function<void()>>({
[&]() { InvokeParser(); },
- [&]() { InvokeInterfaceUtils(); },
[&]() { InvokeLimitParser(); },
});
invoke_parser_fuzzer();
diff --git a/init/host_init_verifier.cpp b/init/host_init_verifier.cpp
index f746ab9..287857a 100644
--- a/init/host_init_verifier.cpp
+++ b/init/host_init_verifier.cpp
@@ -297,9 +297,7 @@
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 @@
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;
}
diff --git a/init/init.cpp b/init/init.cpp
index 4878660..6c80899 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -268,8 +268,8 @@
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 @@
}
}
#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;
diff --git a/init/init_test.cpp b/init/init_test.cpp
index 5088273..f280de9 100644
--- a/init/init_test.cpp
+++ b/init/init_test.cpp
@@ -62,8 +62,7 @@
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 @@
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 @@
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());
diff --git a/init/interface_utils.cpp b/init/interface_utils.cpp
index 1b76bba..84407aa 100644
--- a/init/interface_utils.cpp
+++ b/init/interface_utils.cpp
@@ -39,27 +39,6 @@
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 @@
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) {
diff --git a/init/interface_utils.h b/init/interface_utils.h
index 4ca377f..214feda 100644
--- a/init/interface_utils.h
+++ b/init/interface_utils.h
@@ -34,8 +34,6 @@
// 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.
diff --git a/init/reboot_test.cpp b/init/reboot_test.cpp
index b3d038d..b7a1cfd 100644
--- a/init/reboot_test.cpp
+++ b/init/reboot_test.cpp
@@ -103,8 +103,7 @@
"$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);
diff --git a/init/service_parser.cpp b/init/service_parser.cpp
index 6781c70..e6f3af6 100644
--- a/init/service_parser.cpp
+++ b/init/service_parser.cpp
@@ -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::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 @@
}
}
- 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()
diff --git a/init/service_parser.h b/init/service_parser.h
index 670a5c6..f06cfc4 100644
--- a/init/service_parser.h
+++ b/init/service_parser.h
@@ -18,7 +18,6 @@
#include <vector>
-#include "interface_utils.h"
#include "parser.h"
#include "service.h"
#include "service_list.h"
@@ -29,13 +28,8 @@
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 @@
ServiceList* service_list_;
Subcontext* subcontext_;
- std::optional<InterfaceInheritanceHierarchyMap> interface_inheritance_hierarchy_;
std::unique_ptr<Service> service_;
std::string filename_;
};
diff --git a/init/service_test.cpp b/init/service_test.cpp
index a3590b5..53b53ed 100644
--- a/init/service_test.cpp
+++ b/init/service_test.cpp
@@ -253,8 +253,7 @@
"$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);
diff --git a/init/test_utils/service_utils.cpp b/init/test_utils/service_utils.cpp
index 6426ed9..7002a67 100644
--- a/init/test_utils/service_utils.cpp
+++ b/init/test_utils/service_utils.cpp
@@ -30,8 +30,7 @@
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",