ueventd: remove PlatformDeviceList
In order to create symlinks for USB and block devices, the path for
their parent platform device must be known.
Previously, ueventd would save each platform device that it encounters
to a list and query this list when creating the symlinks. That,
however, is racy because the uevent socket does not differentiate
uevents from device_init() and uevents sent by the kernel when
probing a device first the first time. The below scenario is the
faulty case:
1) Kernel probes parent platform device for a block device
2) ueventd calls device_init() and starts processing uevents
3) Kernel probes block device and sends its uevents
4) ueventd picks up the block device uevent during its uevent processing,
without yet regenerating the platform device uevent, causing improper
symlinks to be created.
This change stops storing the platform devices in a list, and instead
traverses up the directory structure for each USB or block device
until it reaches a platform device, defined as one whose subsystem is
the platform bus. This fixes the race and simplifies the ueventd
code.
Bug: 62436493
Bug: 62681642
Test: Boot bullhead
Test: Boot sailfish
Test: Boot hikey + hotplug/unplug sdcard
Merged-In: I21636355d8e434f30e0cba568598a6cf139e67f9
Change-Id: I21636355d8e434f30e0cba568598a6cf139e67f9
diff --git a/init/devices.cpp b/init/devices.cpp
index 39571ac..27a20a4 100644
--- a/init/devices.cpp
+++ b/init/devices.cpp
@@ -46,6 +46,7 @@
#include <android-base/file.h>
#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <cutils/list.h>
#include <cutils/uevent.h>
@@ -79,16 +80,8 @@
struct listnode plist;
};
-struct platform_node {
- char *name;
- char *path;
- int path_len;
- struct listnode list;
-};
-
static list_declare(sys_perms);
static list_declare(dev_perms);
-static list_declare(platform_names);
int add_dev_perms(const char *name, const char *attr,
mode_t perm, unsigned int uid, unsigned int gid,
@@ -286,77 +279,37 @@
}
}
-static void add_platform_device(const char *path)
-{
- int path_len = strlen(path);
- struct platform_node *bus;
- const char *name = path;
+// 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 FindPlatformDevice(std::string path, std::string* platform_device_path) {
+ platform_device_path->clear();
- if (!strncmp(path, "/devices/", 9)) {
- name += 9;
- if (!strncmp(name, "platform/", 9))
- name += 9;
- }
+ static const std::string kSysfsMountPoint = "/sys";
- LOG(VERBOSE) << "adding platform device " << name << " (" << path << ")";
+ // Uevents don't contain the mount point, so we need to add it here.
+ path.insert(0, kSysfsMountPoint);
- bus = (platform_node*) calloc(1, sizeof(struct platform_node));
- bus->path = strdup(path);
- bus->path_len = path_len;
- bus->name = bus->path + (name - path);
- list_add_tail(&platform_names, &bus->list);
-}
+ std::string directory = android::base::Dirname(path);
-/*
- * 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
- * 0.
- */
-static struct platform_node *find_platform_device(const char *path)
-{
- int path_len = strlen(path);
- struct listnode *node;
- struct platform_node *bus;
-
- list_for_each_reverse(node, &platform_names) {
- bus = node_to_item(node, struct platform_node, list);
- if ((bus->path_len < path_len) &&
- (path[bus->path_len] == '/') &&
- !strncmp(path, bus->path, bus->path_len))
- return bus;
- }
-
- return NULL;
-}
-
-static void remove_platform_device(const char *path)
-{
- struct listnode *node;
- struct platform_node *bus;
-
- list_for_each_reverse(node, &platform_names) {
- bus = node_to_item(node, struct platform_node, list);
- if (!strcmp(path, bus->path)) {
- LOG(INFO) << "removing platform device " << bus->name;
- free(bus->path);
- list_remove(node);
- free(bus);
- return;
+ while (directory != "/" && directory != ".") {
+ std::string subsystem_link_path;
+ if (android::base::Realpath(directory + "/subsystem", &subsystem_link_path) &&
+ subsystem_link_path == kSysfsMountPoint + "/bus/platform") {
+ // We need to remove the mount point that we added above before returning.
+ directory.erase(0, kSysfsMountPoint.size());
+ *platform_device_path = directory;
+ return true;
}
- }
-}
-static void destroy_platform_devices() {
- struct listnode* node;
- struct listnode* n;
- struct platform_node* bus;
+ auto last_slash = path.rfind('/');
+ if (last_slash == std::string::npos) return false;
- list_for_each_safe(node, n, &platform_names) {
- list_remove(node);
- bus = node_to_item(node, struct platform_node, list);
- free(bus->path);
- free(bus);
+ path.erase(last_slash);
+ directory = android::base::Dirname(path);
}
+
+ return false;
}
/* Given a path that may start with a PCI device, populate the supplied buffer
@@ -480,11 +433,9 @@
char **links;
int link_num = 0;
int width;
- struct platform_node *pdev;
- pdev = find_platform_device(uevent->path);
- if (!pdev)
- return NULL;
+ std::string platform_device;
+ if (!FindPlatformDevice(uevent->path, &platform_device)) return nullptr;
links = (char**) malloc(sizeof(char *) * 2);
if (!links)
@@ -492,7 +443,7 @@
memset(links, 0, sizeof(char *) * 2);
/* skip "/devices/platform/<driver>" */
- parent = strchr(uevent->path + pdev->path_len, '/');
+ parent = strchr(uevent->path + platform_device.size(), '/');
if (!parent)
goto err;
@@ -527,8 +478,6 @@
}
char** get_block_device_symlinks(struct uevent* uevent) {
- const char *device;
- struct platform_node *pdev;
const char *slash;
const char *type;
char buf[256];
@@ -536,9 +485,18 @@
int link_num = 0;
char *p;
- pdev = find_platform_device(uevent->path);
- if (pdev) {
- device = pdev->name;
+ std::string 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/";
+
+ if (android::base::StartsWith(device, devices_platform_prefix.c_str())) {
+ device = device.substr(devices_platform_prefix.length());
+ } else if (android::base::StartsWith(device, devices_prefix.c_str())) {
+ device = device.substr(devices_prefix.length());
+ }
+
type = "platform";
} else if (!find_pci_device_prefix(uevent->path, buf, sizeof(buf))) {
device = buf;
@@ -557,7 +515,7 @@
LOG(VERBOSE) << "found " << type << " device " << device;
- snprintf(link_path, sizeof(link_path), "/dev/block/%s/%s", type, device);
+ snprintf(link_path, sizeof(link_path), "/dev/block/%s/%s", type, device.c_str());
if (uevent->partition_name) {
p = strdup(uevent->partition_name);
@@ -635,16 +593,6 @@
}
}
-static void handle_platform_device_event(struct uevent *uevent)
-{
- const char *path = uevent->path;
-
- if (!strcmp(uevent->action, "add"))
- add_platform_device(path);
- else if (!strcmp(uevent->action, "remove"))
- remove_platform_device(path);
-}
-
static const char *parse_device_name(struct uevent *uevent, unsigned int len)
{
const char *name;
@@ -824,8 +772,6 @@
if (!strncmp(uevent->subsystem, "block", 5)) {
handle_block_device_event(uevent);
- } else if (!strncmp(uevent->subsystem, "platform", 8)) {
- handle_platform_device_event(uevent);
} else {
handle_generic_device_event(uevent);
}
@@ -1078,7 +1024,6 @@
}
void device_close() {
- destroy_platform_devices();
device_fd.reset();
selinux_status_close();
}
diff --git a/init/init_first_stage.cpp b/init/init_first_stage.cpp
index bcc8d1b..60ce6e9 100644
--- a/init/init_first_stage.cpp
+++ b/init/init_first_stage.cpp
@@ -178,11 +178,6 @@
}
coldboot_action_t FirstStageMount::ColdbootCallback(uevent* uevent) {
- // We need platform devices to create symlinks.
- if (!strncmp(uevent->subsystem, "platform", 8)) {
- return COLDBOOT_CREATE;
- }
-
// Ignores everything that is not a block device.
if (strncmp(uevent->subsystem, "block", 5)) {
return COLDBOOT_CONTINUE;