diff --git a/init/devices.cpp b/init/devices.cpp index c52d8f860..2943fb732 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -147,21 +147,34 @@ void SysfsPermissions::SetPermissions(const std::string& path) const { } } -// Given a path that may start with a platform device, find the length of the -// platform device prefix. If it doesn't start with a platform device, return false -bool PlatformDeviceList::Find(const std::string& path, std::string* out_path) const { - out_path->clear(); - // platform_devices is searched backwards, since parents are added before their children, - // and we want to match as deep of a child as we can. - for (auto it = platform_devices_.crbegin(); it != platform_devices_.crend(); ++it) { - auto platform_device_path_length = it->length(); - if (platform_device_path_length < path.length() && - path[platform_device_path_length] == '/' && - android::base::StartsWith(path, it->c_str())) { - *out_path = *it; +// Given a path that may start with a platform device, find the parent platform device by finding a +// parent directory with a 'subsystem' symlink that points to the platform bus. +// If it doesn't start with a platform device, return false +bool DeviceHandler::FindPlatformDevice(std::string path, std::string* platform_device_path) const { + platform_device_path->clear(); + + // Uevents don't contain the mount point, so we need to add it here. + path.insert(0, sysfs_mount_point_); + + std::string directory = android::base::Dirname(path); + + while (directory != "/" && directory != ".") { + std::string subsystem_link_path; + if (android::base::Realpath(directory + "/subsystem", &subsystem_link_path) && + subsystem_link_path == sysfs_mount_point_ + "/bus/platform") { + // We need to remove the mount point that we added above before returning. + directory.erase(0, sysfs_mount_point_.size()); + *platform_device_path = directory; return true; } + + auto last_slash = path.rfind('/'); + if (last_slash == std::string::npos) return false; + + path.erase(last_slash); + directory = android::base::Dirname(path); } + return false; } @@ -258,7 +271,7 @@ out: std::vector DeviceHandler::GetCharacterDeviceSymlinks(const Uevent& uevent) const { std::string parent_device; - if (!platform_devices_.Find(uevent.path, &parent_device)) return {}; + if (!FindPlatformDevice(uevent.path, &parent_device)) return {}; // skip path to the parent driver std::string path = uevent.path.substr(parent_device.length()); @@ -316,7 +329,7 @@ std::vector DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev std::string device; std::string type; - if (platform_devices_.Find(uevent.path, &device)) { + if (FindPlatformDevice(uevent.path, &device)) { // Skip /devices/platform or /devices/ if present static const std::string devices_platform_prefix = "/devices/platform/"; static const std::string devices_prefix = "/devices/"; @@ -388,14 +401,6 @@ void DeviceHandler::HandleDevice(const std::string& action, const std::string& d } } -void DeviceHandler::HandlePlatformDeviceEvent(const Uevent& uevent) { - if (uevent.action == "add") { - platform_devices_.Add(uevent.path); - } else if (uevent.action == "remove") { - platform_devices_.Remove(uevent.path); - } -} - void DeviceHandler::HandleBlockDeviceEvent(const Uevent& uevent) const { // if it's not a /dev device, nothing to do if (uevent.major < 0 || uevent.minor < 0) return; @@ -458,8 +463,6 @@ void DeviceHandler::HandleDeviceEvent(const Uevent& uevent) { if (uevent.subsystem == "block") { HandleBlockDeviceEvent(uevent); - } else if (uevent.subsystem == "platform") { - HandlePlatformDeviceEvent(uevent); } else { HandleGenericDeviceEvent(uevent); } @@ -472,7 +475,8 @@ DeviceHandler::DeviceHandler(std::vector dev_permissions, sysfs_permissions_(std::move(sysfs_permissions)), subsystems_(std::move(subsystems)), sehandle_(selinux_android_file_context_handle()), - skip_restorecon_(skip_restorecon) {} + skip_restorecon_(skip_restorecon), + sysfs_mount_point_("/sys") {} DeviceHandler::DeviceHandler() : DeviceHandler(std::vector{}, std::vector{}, diff --git a/init/devices.h b/init/devices.h index 09a0ce3ee..362c38cb8 100644 --- a/init/devices.h +++ b/init/devices.h @@ -93,20 +93,6 @@ class Subsystem { DevnameSource devname_source_; }; -class PlatformDeviceList { - public: - void Add(const std::string& path) { platform_devices_.emplace_back(path); } - void Remove(const std::string& path) { - auto it = std::find(platform_devices_.begin(), platform_devices_.end(), path); - if (it != platform_devices_.end()) platform_devices_.erase(it); - } - bool Find(const std::string& path, std::string* out_path) const; - auto size() const { return platform_devices_.size(); } - - private: - std::vector platform_devices_; -}; - class DeviceHandler { public: friend class DeviceHandlerTester; @@ -119,16 +105,11 @@ class DeviceHandler { void HandleDeviceEvent(const Uevent& uevent); - void FixupSysPermissions(const std::string& upath, const std::string& subsystem) const; - - void HandlePlatformDeviceEvent(const Uevent& uevent); - void HandleBlockDeviceEvent(const Uevent& uevent) const; - void HandleGenericDeviceEvent(const Uevent& uevent) const; - std::vector GetBlockDeviceSymlinks(const Uevent& uevent) const; void set_skip_restorecon(bool value) { skip_restorecon_ = value; } private: + bool FindPlatformDevice(std::string path, std::string* platform_device_path) const; std::tuple GetDevicePermissions( const std::string& path, const std::vector& links) const; void MakeDevice(const std::string& path, int block, int major, int minor, @@ -136,13 +117,17 @@ class DeviceHandler { std::vector GetCharacterDeviceSymlinks(const Uevent& uevent) const; void HandleDevice(const std::string& action, const std::string& devpath, int block, int major, int minor, const std::vector& links) const; + void FixupSysPermissions(const std::string& upath, const std::string& subsystem) const; + + void HandleBlockDeviceEvent(const Uevent& uevent) const; + void HandleGenericDeviceEvent(const Uevent& uevent) const; std::vector dev_permissions_; std::vector sysfs_permissions_; std::vector subsystems_; - PlatformDeviceList platform_devices_; selabel_handle* sehandle_; bool skip_restorecon_; + std::string sysfs_mount_point_; }; // Exposed for testing diff --git a/init/devices_test.cpp b/init/devices_test.cpp index 41b101b22..e1e4e49f6 100644 --- a/init/devices_test.cpp +++ b/init/devices_test.cpp @@ -16,33 +16,29 @@ #include "devices.h" -#include -#include - #include +#include #include +#include "util.h" + +using namespace std::string_literals; + class DeviceHandlerTester { public: - void AddPlatformDevice(const std::string& path) { - Uevent uevent = { - .action = "add", .subsystem = "platform", .path = path, - }; - device_handler_.HandlePlatformDeviceEvent(uevent); - } - - void RemovePlatformDevice(const std::string& path) { - Uevent uevent = { - .action = "remove", .subsystem = "platform", .path = path, - }; - device_handler_.HandlePlatformDeviceEvent(uevent); - } - - void TestGetSymlinks(const std::string& platform_device_name, const Uevent& uevent, + void TestGetSymlinks(const std::string& platform_device, const Uevent& uevent, const std::vector expected_links, bool block) { - AddPlatformDevice(platform_device_name); - auto platform_device_remover = android::base::make_scope_guard( - [this, &platform_device_name]() { RemovePlatformDevice(platform_device_name); }); + TemporaryDir fake_sys_root; + device_handler_.sysfs_mount_point_ = fake_sys_root.path; + + std::string platform_device_dir = fake_sys_root.path + platform_device; + mkdir_recursive(platform_device_dir, 0777, nullptr); + + std::string platform_bus = fake_sys_root.path + "/bus/platform"s; + mkdir_recursive(platform_bus, 0777, nullptr); + symlink(platform_bus.c_str(), (platform_device_dir + "/subsystem").c_str()); + + mkdir_recursive(android::base::Dirname(fake_sys_root.path + uevent.path), 0777, nullptr); std::vector result; if (block) { @@ -65,30 +61,6 @@ class DeviceHandlerTester { DeviceHandler device_handler_; }; -TEST(device_handler, PlatformDeviceList) { - PlatformDeviceList platform_device_list; - - platform_device_list.Add("/devices/platform/some_device_name"); - platform_device_list.Add("/devices/platform/some_device_name/longer"); - platform_device_list.Add("/devices/platform/other_device_name"); - EXPECT_EQ(3U, platform_device_list.size()); - - std::string out_path; - EXPECT_FALSE(platform_device_list.Find("/devices/platform/not_found", &out_path)); - EXPECT_EQ("", out_path); - - EXPECT_FALSE(platform_device_list.Find("/devices/platform/some_device_name_with_same_prefix", - &out_path)); - - EXPECT_TRUE(platform_device_list.Find("/devices/platform/some_device_name/longer/longer_child", - &out_path)); - EXPECT_EQ("/devices/platform/some_device_name/longer", out_path); - - EXPECT_TRUE( - platform_device_list.Find("/devices/platform/some_device_name/other_child", &out_path)); - EXPECT_EQ("/devices/platform/some_device_name", out_path); -} - TEST(device_handler, get_character_device_symlinks_success) { const char* platform_device = "/devices/platform/some_device_name"; Uevent uevent = { diff --git a/init/init_first_stage.cpp b/init/init_first_stage.cpp index 0f2b1f3d4..ebbe1cd18 100644 --- a/init/init_first_stage.cpp +++ b/init/init_first_stage.cpp @@ -181,12 +181,6 @@ void FirstStageMount::InitRequiredDevices() { } RegenerationAction FirstStageMount::UeventCallback(const Uevent& uevent) { - // We need platform devices to create symlinks. - if (uevent.subsystem == "platform") { - device_handler_.HandleDeviceEvent(uevent); - return RegenerationAction::kContinue; - } - // Ignores everything that is not a block device. if (uevent.subsystem != "block") { return RegenerationAction::kContinue; diff --git a/init/ueventd.cpp b/init/ueventd.cpp index d12164719..28c1c076a 100644 --- a/init/ueventd.cpp +++ b/init/ueventd.cpp @@ -128,15 +128,7 @@ class ColdBoot { void ColdBoot::UeventHandlerMain(unsigned int process_num, unsigned int total_processes) { for (unsigned int i = process_num; i < uevent_queue_.size(); i += total_processes) { auto& uevent = uevent_queue_[i]; - if (uevent.action == "add" || uevent.action == "change" || uevent.action == "online") { - device_handler_.FixupSysPermissions(uevent.path, uevent.subsystem); - } - - if (uevent.subsystem == "block") { - device_handler_.HandleBlockDeviceEvent(uevent); - } else { - device_handler_.HandleGenericDeviceEvent(uevent); - } + device_handler_.HandleDeviceEvent(uevent); } _exit(EXIT_SUCCESS); } @@ -145,14 +137,6 @@ void ColdBoot::RegenerateUevents() { uevent_listener_.RegenerateUevents([this](const Uevent& uevent) { HandleFirmwareEvent(uevent); - // This is the one mutable part of DeviceHandler, in which platform devices are - // added to a vector for later reference. Since there is no communication after - // fork()'ing subprocess handlers, all platform devices must be in the vector before - // we fork, and therefore they must be handled in this loop. - if (uevent.subsystem == "platform") { - device_handler_.HandlePlatformDeviceEvent(uevent); - } - uevent_queue_.emplace_back(std::move(uevent)); return RegenerationAction::kContinue; });