libdm: Improve the reliability of dm device paths.
This fixes a race condition where WaitForFile() after
GetDmDevicePathByName appears to succeed, but a subsequent operation on
the path fails. This can happen when CreateDevice() is called
immediately after a call to DeleteDevice (from any process), and the
path is re-used, enqueuing udev events to remove and re-add the block
device.
The fix for this is to introduce a new variant of CreateDevice() that
has a timeout parameter. When the timeout is positive, CreateDevice()
will wait for a /dev/block/mapper/by-uuid symlink to be created, which
signals that ueventd has finished processing the operation.
ueventd will now create these by-uuid symlinks for device-mapper nodes.
Unfortunately, the uuid is only available during "change" events, so we
have to special case device-mapper symlink creation. And since the uuid
is not available during "remove" events, we simply find matching links
to remove them.
This ensures that callers of CreateDevice() can use the device path
knowing that no asynchronous removals are pending. Code that uses the
old CreateDevice+WaitForFile pattern will be transitioned to the new
method.
Note that it is safe to ignore the timeout, or to use the "unsafe"
CreateDevice, if the caller ensures the path by other means. For example
first-stage init has no device removal, and regenerates uevents until
it has acquired all the paths it needs.
Finally, since libdm now inspects sysfs unconditionally, libdm consumers
need r_dir_file perms for sysfs_dm in their sepolicy. Additionally
linking to libdm now requires linking to libext2_uuid.
Bug: 135771280
Test: libdm_test
device flashes, boots
Change-Id: If5a7383ea38f32a7fbbcf24842dce6a668050a70
diff --git a/init/devices.cpp b/init/devices.cpp
index e8e6cd7..f6e453a 100644
--- a/init/devices.cpp
+++ b/init/devices.cpp
@@ -22,7 +22,6 @@
#include <unistd.h>
#include <chrono>
-#include <map>
#include <memory>
#include <string>
#include <thread>
@@ -111,24 +110,16 @@
// the supplied buffer with the dm module's instantiated name.
// If it doesn't start with a virtual block device, or there is some
// error, return false.
-static bool FindDmDevicePartition(const std::string& path, std::string* result) {
- result->clear();
+static bool FindDmDevice(const std::string& path, std::string* name, std::string* uuid) {
if (!StartsWith(path, "/devices/virtual/block/dm-")) return false;
- if (getpid() == 1) return false; // first_stage_init has no sepolicy needs
- static std::map<std::string, std::string> cache;
- // wait_for_file will not work, the content is also delayed ...
- for (android::base::Timer t; t.duration() < 200ms; std::this_thread::sleep_for(10ms)) {
- if (ReadFileToString("/sys" + path + "/dm/name", result) && !result->empty()) {
- // Got it, set cache with result, when node arrives
- cache[path] = *result = Trim(*result);
- return true;
- }
+ if (!ReadFileToString("/sys" + path + "/dm/name", name)) {
+ return false;
}
- auto it = cache.find(path);
- if ((it == cache.end()) || (it->second.empty())) return false;
- // Return cached results, when node goes away
- *result = it->second;
+ ReadFileToString("/sys" + path + "/dm/uuid", uuid);
+
+ *name = android::base::Trim(*name);
+ *uuid = android::base::Trim(*uuid);
return true;
}
@@ -325,6 +316,7 @@
std::string device;
std::string type;
std::string partition;
+ std::string uuid;
if (FindPlatformDevice(uevent.path, &device)) {
// Skip /devices/platform or /devices/ if present
@@ -342,8 +334,12 @@
type = "pci";
} else if (FindVbdDevicePrefix(uevent.path, &device)) {
type = "vbd";
- } else if (FindDmDevicePartition(uevent.path, &partition)) {
- return {"/dev/block/mapper/" + partition};
+ } else if (FindDmDevice(uevent.path, &partition, &uuid)) {
+ std::vector<std::string> symlinks = {"/dev/block/mapper/" + partition};
+ if (!uuid.empty()) {
+ symlinks.emplace_back("/dev/block/mapper/by-uuid/" + uuid);
+ }
+ return symlinks;
} else {
return {};
}
@@ -379,10 +375,41 @@
return links;
}
+static void RemoveDeviceMapperLinks(const std::string& devpath) {
+ std::vector<std::string> dirs = {
+ "/dev/block/mapper",
+ "/dev/block/mapper/by-uuid",
+ };
+ for (const auto& dir : dirs) {
+ if (access(dir.c_str(), F_OK) != 0) continue;
+
+ std::unique_ptr<DIR, decltype(&closedir)> dh(opendir(dir.c_str()), closedir);
+ if (!dh) {
+ PLOG(ERROR) << "Failed to open directory " << dir;
+ continue;
+ }
+
+ struct dirent* dp;
+ std::string link_path;
+ while ((dp = readdir(dh.get())) != nullptr) {
+ if (dp->d_type != DT_LNK) continue;
+
+ auto path = dir + "/" + dp->d_name;
+ if (Readlink(path, &link_path) && link_path == devpath) {
+ unlink(path.c_str());
+ }
+ }
+ }
+}
+
void DeviceHandler::HandleDevice(const std::string& action, const std::string& devpath, bool block,
int major, int minor, const std::vector<std::string>& links) const {
if (action == "add") {
MakeDevice(devpath, block, major, minor, links);
+ }
+
+ // We don't have full device-mapper information until a change event is fired.
+ if (action == "add" || (action == "change" && StartsWith(devpath, "/dev/block/dm-"))) {
for (const auto& link : links) {
if (!mkdir_recursive(Dirname(link), 0755)) {
PLOG(ERROR) << "Failed to create directory " << Dirname(link);
@@ -401,6 +428,9 @@
}
if (action == "remove") {
+ if (StartsWith(devpath, "/dev/block/dm-")) {
+ RemoveDeviceMapperLinks(devpath);
+ }
for (const auto& link : links) {
std::string link_path;
if (Readlink(link, &link_path) && link_path == devpath) {