commit 95c4242cf6eb67b0ae36aeb3e15325919765e3df
author Jooyung Han <jooyung@google.com> Wed Aug 21 13:57:44 2024 +0900
committer Jooyung Han <jooyung@google.com> Wed Aug 21 17:42:05 2024 +0900
tree 6d47e93a8e6f6ffd9a6a262b0e4d835106cfd45c
parent d6790c4bc64bfd7145b8a14b45e9e3f1a3db6c62

host_init_verifier: check interface names directly

Previously, ServiceParser did the check, but only when it's invoked by
host_init_verifier. Host_init_verifier can do it directly, which removes
unnecessary runtime dependencies from init.

Bug: 326827772
Test: host_init_verifier detects wrong HIDL interface names.
Change-Id: I4c8bb0e89a5def7341c48c52af730795a6ee13c0

init/host_init_verifier.cpp

diff --git a/init/host_init_verifier.cpp b/init/host_init_verifier.cpp
index f746ab9..52be0a1 100644
--- a/init/host_init_verifier.cpp
+++ b/init/host_init_verifier.cpp
@@ -298,8 +298,7 @@
     ServiceList& sl = ServiceList::GetInstance();
     Parser parser;
     parser.AddSectionParser("service",
-                            std::make_unique<ServiceParser>(&sl, GetSubcontext(),
-                                                            *interface_inheritance_hierarchy_map));
+                            std::make_unique<ServiceParser>(&sl, GetSubcontext(), std::nullopt));
     parser.AddSectionParser("on", std::make_unique<ActionParser>(&am, GetSubcontext()));
     parser.AddSectionParser("import", std::make_unique<HostImportParser>());
 
@@ -317,11 +316,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;
 }
 

init/interface_utils.cpp

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) {

init/interface_utils.h

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.