Merge "ueventd: replace char** links with std::vector<std::string>"

This commit is contained in:
Treehugger Robot 2017-04-12 00:57:34 +00:00 committed by Gerrit Code Review
commit 162118928e
3 changed files with 152 additions and 196 deletions

View file

@ -32,12 +32,16 @@
#include <sys/wait.h> #include <sys/wait.h>
#include <unistd.h> #include <unistd.h>
#include <algorithm>
#include <memory> #include <memory>
#include <string>
#include <thread> #include <thread>
#include <vector>
#include <android-base/file.h> #include <android-base/file.h>
#include <android-base/logging.h> #include <android-base/logging.h>
#include <android-base/stringprintf.h> #include <android-base/stringprintf.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h> #include <android-base/unique_fd.h>
#include <cutils/list.h> #include <cutils/list.h>
#include <cutils/uevent.h> #include <cutils/uevent.h>
@ -178,9 +182,8 @@ static void fixup_sys_perms(const char* upath, const char* subsystem) {
} }
} }
static mode_t get_device_perm(const char *path, const char **links, static mode_t get_device_perm(const char* path, const std::vector<std::string>& links,
unsigned *uid, unsigned *gid) unsigned* uid, unsigned* gid) {
{
struct listnode *node; struct listnode *node;
struct perm_node *perm_node; struct perm_node *perm_node;
struct perms_ *dp; struct perms_ *dp;
@ -189,26 +192,12 @@ static mode_t get_device_perm(const char *path, const char **links,
* override ueventd.rc * override ueventd.rc
*/ */
list_for_each_reverse(node, &dev_perms) { list_for_each_reverse(node, &dev_perms) {
bool match = false;
perm_node = node_to_item(node, struct perm_node, plist); perm_node = node_to_item(node, struct perm_node, plist);
dp = &perm_node->dp; dp = &perm_node->dp;
if (perm_path_matches(path, dp)) { if (perm_path_matches(path, dp) ||
match = true; std::any_of(links.begin(), links.end(),
} else { [dp](const auto& link) { return perm_path_matches(link.c_str(), dp); })) {
if (links) {
int i;
for (i = 0; links[i]; i++) {
if (perm_path_matches(links[i], dp)) {
match = true;
break;
}
}
}
}
if (match) {
*uid = dp->uid; *uid = dp->uid;
*gid = dp->gid; *gid = dp->gid;
return dp->perm; return dp->perm;
@ -220,11 +209,8 @@ static mode_t get_device_perm(const char *path, const char **links,
return 0600; return 0600;
} }
static void make_device(const char *path, static void make_device(const char* path, const char* /*upath*/, int block, int major, int minor,
const char */*upath*/, const std::vector<std::string>& links) {
int block, int major, int minor,
const char **links)
{
unsigned uid; unsigned uid;
unsigned gid; unsigned gid;
mode_t mode; mode_t mode;
@ -234,7 +220,12 @@ static void make_device(const char *path,
mode = get_device_perm(path, links, &uid, &gid) | (block ? S_IFBLK : S_IFCHR); mode = get_device_perm(path, links, &uid, &gid) | (block ? S_IFBLK : S_IFCHR);
if (sehandle) { if (sehandle) {
if (selabel_lookup_best_match(sehandle, &secontext, path, links, mode)) { std::vector<const char*> c_links;
for (const auto& link : links) {
c_links.emplace_back(link.c_str());
}
c_links.emplace_back(nullptr);
if (selabel_lookup_best_match(sehandle, &secontext, path, &c_links[0], mode)) {
PLOG(ERROR) << "Device '" << path << "' not created; cannot find SELinux label"; PLOG(ERROR) << "Device '" << path << "' not created; cannot find SELinux label";
return; return;
} }
@ -356,60 +347,55 @@ static void destroy_platform_devices() {
/* Given a path that may start with a PCI device, populate the supplied buffer /* Given a path that may start with a PCI device, populate the supplied buffer
* with the PCI domain/bus number and the peripheral ID and return 0. * with the PCI domain/bus number and the peripheral ID and return 0.
* If it doesn't start with a PCI device, or there is some error, return -1 */ * If it doesn't start with a PCI device, or there is some error, return -1 */
static int find_pci_device_prefix(const char *path, char *buf, ssize_t buf_sz) static bool find_pci_device_prefix(const std::string& path, std::string* result) {
{ result->clear();
const char *start, *end;
if (strncmp(path, "/devices/pci", 12)) if (!android::base::StartsWith(path, "/devices/pci")) return false;
return -1;
/* Beginning of the prefix is the initial "pci" after "/devices/" */ /* Beginning of the prefix is the initial "pci" after "/devices/" */
start = path + 9; std::string::size_type start = 9;
/* End of the prefix is two path '/' later, capturing the domain/bus number /* End of the prefix is two path '/' later, capturing the domain/bus number
* and the peripheral ID. Example: pci0000:00/0000:00:1f.2 */ * and the peripheral ID. Example: pci0000:00/0000:00:1f.2 */
end = strchr(start, '/'); auto end = path.find('/', start);
if (!end) if (end == std::string::npos) return false;
return -1;
end = strchr(end + 1, '/');
if (!end)
return -1;
/* Make sure we have enough room for the string plus null terminator */ end = path.find('/', end + 1);
if (end - start + 1 > buf_sz) if (end == std::string::npos) return false;
return -1;
strncpy(buf, start, end - start); auto length = end - start;
buf[end - start] = '\0'; if (length <= 4) {
return 0; // The minimum string that will get to this check is 'pci/', which is malformed,
// so return false
return false;
}
*result = path.substr(start, length);
return true;
} }
/* Given a path that may start with a virtual block device, populate /* Given a path that may start with a virtual block device, populate
* the supplied buffer with the virtual block device ID and return 0. * the supplied buffer with the virtual block device ID and return 0.
* If it doesn't start with a virtual block device, or there is some * If it doesn't start with a virtual block device, or there is some
* error, return -1 */ * error, return -1 */
static int find_vbd_device_prefix(const char *path, char *buf, ssize_t buf_sz) static bool find_vbd_device_prefix(const std::string& path, std::string* result) {
{ result->clear();
const char *start, *end;
if (!android::base::StartsWith(path, "/devices/vbd-")) return false;
/* Beginning of the prefix is the initial "vbd-" after "/devices/" */ /* Beginning of the prefix is the initial "vbd-" after "/devices/" */
if (strncmp(path, "/devices/vbd-", 13)) std::string::size_type start = 13;
return -1;
/* End of the prefix is one path '/' later, capturing the /* End of the prefix is one path '/' later, capturing the
virtual block device ID. Example: 768 */ virtual block device ID. Example: 768 */
start = path + 13; auto end = path.find('/', start);
end = strchr(start, '/'); if (end == std::string::npos) return false;
if (!end)
return -1;
/* Make sure we have enough room for the string plus null terminator */ auto length = end - start;
if (end - start + 1 > buf_sz) if (length == 0) return false;
return -1;
strncpy(buf, start, end - start); *result = path.substr(start, length);
buf[end - start] = '\0'; return true;
return 0;
} }
static void parse_event(const char *msg, struct uevent *uevent) static void parse_event(const char *msg, struct uevent *uevent)
@ -467,133 +453,108 @@ static void parse_event(const char *msg, struct uevent *uevent)
} }
} }
char** get_character_device_symlinks(struct uevent* uevent) { std::vector<std::string> get_character_device_symlinks(uevent* uevent) {
const char *parent; platform_node* pdev = find_platform_device(uevent->path);
const char *slash; if (!pdev) return {};
char **links;
int link_num = 0;
int width;
struct platform_node *pdev;
pdev = find_platform_device(uevent->path);
if (!pdev)
return NULL;
links = (char**) malloc(sizeof(char *) * 2);
if (!links)
return NULL;
memset(links, 0, sizeof(char *) * 2);
/* skip "/devices/platform/<driver>" */ /* skip "/devices/platform/<driver>" */
parent = strchr(uevent->path + pdev->path_len, '/'); std::string parent = std::string(uevent->path);
if (!parent) auto parent_start = parent.find('/', pdev->path_len);
goto err; if (parent_start == std::string::npos) return {};
if (!strncmp(parent, "/usb", 4)) { parent.erase(0, parent_start);
/* skip root hub name and device. use device interface */
while (*++parent && *parent != '/');
if (*parent)
while (*++parent && *parent != '/');
if (!*parent)
goto err;
slash = strchr(++parent, '/');
if (!slash)
goto err;
width = slash - parent;
if (width <= 0)
goto err;
if (asprintf(&links[link_num], "/dev/usb/%s%.*s", uevent->subsystem, width, parent) > 0) if (!android::base::StartsWith(parent, "/usb")) return {};
link_num++;
else // skip root hub name and device. use device interface
links[link_num] = NULL; // skip 3 slashes, including the first / by starting the search at the 1st character, not 0th.
mkdir("/dev/usb", 0755); // then extract what comes between the 3rd and 4th slash
} // e.g. "/usb/usb_device/name/tty2-1:1.0" -> "name"
else {
goto err; std::string::size_type start = 0;
} start = parent.find('/', start + 1);
if (start == std::string::npos) return {};
start = parent.find('/', start + 1);
if (start == std::string::npos) return {};
auto end = parent.find('/', start + 1);
if (end == std::string::npos) return {};
start++; // Skip the first '/'
auto length = end - start;
if (length == 0) return {};
auto name_string = parent.substr(start, length);
// TODO: remove std::string() when uevent->subsystem is an std::string
std::vector<std::string> links;
links.emplace_back("/dev/usb/" + std::string(uevent->subsystem) + name_string);
mkdir("/dev/usb", 0755);
return links; return links;
err:
free(links);
return NULL;
} }
// replaces any unacceptable characters with '_', the // replaces any unacceptable characters with '_', the
// length of the resulting string is equal to the input string // length of the resulting string is equal to the input string
void sanitize_partition_name(char* s) { void sanitize_partition_name(std::string* string) {
const char* accept = const char* accept =
"abcdefghijklmnopqrstuvwxyz" "abcdefghijklmnopqrstuvwxyz"
"ABCDEFGHIJKLMNOPQRSTUVWXYZ" "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
"0123456789" "0123456789"
"_-."; "_-.";
if (!s) return; if (!string) return;
while (*s) { std::string::size_type pos = 0;
s += strspn(s, accept); while ((pos = string->find_first_not_of(accept, pos)) != std::string::npos) {
if (*s) *s++ = '_'; (*string)[pos] = '_';
} }
} }
char** get_block_device_symlinks(struct uevent* uevent) { std::vector<std::string> get_block_device_symlinks(uevent* uevent) {
const char *device; std::string device;
struct platform_node *pdev; struct platform_node* pdev;
const char *slash; std::string type;
const char *type;
char buf[256];
char link_path[256];
int link_num = 0;
char *p;
pdev = find_platform_device(uevent->path); pdev = find_platform_device(uevent->path);
if (pdev) { if (pdev) {
device = pdev->name; device = pdev->name;
type = "platform"; type = "platform";
} else if (!find_pci_device_prefix(uevent->path, buf, sizeof(buf))) { } else if (find_pci_device_prefix(uevent->path, &device)) {
device = buf;
type = "pci"; type = "pci";
} else if (!find_vbd_device_prefix(uevent->path, buf, sizeof(buf))) { } else if (find_vbd_device_prefix(uevent->path, &device)) {
device = buf;
type = "vbd"; type = "vbd";
} else { } else {
return NULL; return {};
} }
char **links = (char**) malloc(sizeof(char *) * 4); std::vector<std::string> links;
if (!links)
return NULL;
memset(links, 0, sizeof(char *) * 4);
LOG(VERBOSE) << "found " << type << " device " << device; LOG(VERBOSE) << "found " << type << " device " << device;
snprintf(link_path, sizeof(link_path), "/dev/block/%s/%s", type, device); auto link_path = "/dev/block/" + type + "/" + device;
if (uevent->partition_name) { if (uevent->partition_name) {
p = strdup(uevent->partition_name); std::string partition_name_sanitized(uevent->partition_name);
sanitize_partition_name(p); sanitize_partition_name(&partition_name_sanitized);
if (strcmp(uevent->partition_name, p)) { if (partition_name_sanitized != uevent->partition_name) {
LOG(VERBOSE) << "Linking partition '" << uevent->partition_name << "' as '" << p << "'"; LOG(VERBOSE) << "Linking partition '" << uevent->partition_name << "' as '"
<< partition_name_sanitized << "'";
} }
if (asprintf(&links[link_num], "%s/by-name/%s", link_path, p) > 0) links.emplace_back(link_path + "/by-name/" + partition_name_sanitized);
link_num++;
else
links[link_num] = NULL;
free(p);
} }
if (uevent->partition_num >= 0) { if (uevent->partition_num >= 0) {
if (asprintf(&links[link_num], "%s/by-num/p%d", link_path, uevent->partition_num) > 0) links.emplace_back(link_path + "/by-num/p" + std::to_string(uevent->partition_num));
link_num++;
else
links[link_num] = NULL;
} }
slash = strrchr(uevent->path, '/'); // TODO: remove uevent_path when uevent->path is an std::string
if (asprintf(&links[link_num], "%s/%s", link_path, slash + 1) > 0) std::string uevent_path = uevent->path;
link_num++; auto last_slash = uevent_path.rfind('/');
else links.emplace_back(link_path + "/" + uevent_path.substr(last_slash + 1));
links[link_num] = NULL;
return links; return links;
} }
@ -616,33 +577,21 @@ static void remove_link(const char* oldpath, const char* newpath) {
if (android::base::Readlink(newpath, &path) && path == oldpath) unlink(newpath); if (android::base::Readlink(newpath, &path) && path == oldpath) unlink(newpath);
} }
static void handle_device(const char *action, const char *devpath, static void handle_device(const char* action, const char* devpath, const char* path, int block,
const char *path, int block, int major, int minor, char **links) int major, int minor, const std::vector<std::string>& links) {
{
if(!strcmp(action, "add")) { if(!strcmp(action, "add")) {
make_device(devpath, path, block, major, minor, (const char **)links); make_device(devpath, path, block, major, minor, links);
if (links) { for (const auto& link : links) {
for (int i = 0; links[i]; i++) { make_link_init(devpath, link.c_str());
make_link_init(devpath, links[i]);
}
} }
} }
if(!strcmp(action, "remove")) { if(!strcmp(action, "remove")) {
if (links) { for (const auto& link : links) {
for (int i = 0; links[i]; i++) { remove_link(devpath, link.c_str());
remove_link(devpath, links[i]);
}
} }
unlink(devpath); unlink(devpath);
} }
if (links) {
for (int i = 0; links[i]; i++) {
free(links[i]);
}
free(links);
}
} }
static void handle_platform_device_event(struct uevent *uevent) static void handle_platform_device_event(struct uevent *uevent)
@ -686,7 +635,6 @@ static void handle_block_device_event(struct uevent *uevent)
const char *base = "/dev/block/"; const char *base = "/dev/block/";
const char *name; const char *name;
char devpath[DEVPATH_LEN]; char devpath[DEVPATH_LEN];
char **links = NULL;
name = parse_device_name(uevent, MAX_DEV_NAME); name = parse_device_name(uevent, MAX_DEV_NAME);
if (!name) if (!name)
@ -695,6 +643,7 @@ static void handle_block_device_event(struct uevent *uevent)
snprintf(devpath, sizeof(devpath), "%s%s", base, name); snprintf(devpath, sizeof(devpath), "%s%s", base, name);
make_dir(base, 0755); make_dir(base, 0755);
std::vector<std::string> links;
if (!strncmp(uevent->path, "/devices/", 9)) if (!strncmp(uevent->path, "/devices/", 9))
links = get_block_device_symlinks(uevent); links = get_block_device_symlinks(uevent);
@ -733,7 +682,6 @@ static void handle_generic_device_event(struct uevent *uevent)
const char *base; const char *base;
const char *name; const char *name;
char devpath[DEVPATH_LEN] = {0}; char devpath[DEVPATH_LEN] = {0};
char **links = NULL;
name = parse_device_name(uevent, MAX_DEV_NAME); name = parse_device_name(uevent, MAX_DEV_NAME);
if (!name) if (!name)
@ -818,7 +766,7 @@ static void handle_generic_device_event(struct uevent *uevent)
name += 4; name += 4;
} else } else
base = "/dev/"; base = "/dev/";
links = get_character_device_symlinks(uevent); auto links = get_character_device_symlinks(uevent);
if (!devpath[0]) if (!devpath[0])
snprintf(devpath, sizeof(devpath), "%s%s", base, name); snprintf(devpath, sizeof(devpath), "%s%s", base, name);

View file

@ -20,6 +20,8 @@
#include <sys/stat.h> #include <sys/stat.h>
#include <functional> #include <functional>
#include <string>
#include <vector>
enum coldboot_action_t { enum coldboot_action_t {
// coldboot continues without creating the device for the uevent // coldboot continues without creating the device for the uevent
@ -59,8 +61,8 @@ int get_device_fd();
// Exposed for testing // Exposed for testing
void add_platform_device(const char* path); void add_platform_device(const char* path);
void remove_platform_device(const char* path); void remove_platform_device(const char* path);
char** get_character_device_symlinks(uevent* uevent); std::vector<std::string> get_character_device_symlinks(uevent* uevent);
char** get_block_device_symlinks(struct uevent* uevent); std::vector<std::string> get_block_device_symlinks(uevent* uevent);
void sanitize_partition_name(char* s); void sanitize_partition_name(std::string* string);
#endif /* _INIT_DEVICES_H */ #endif /* _INIT_DEVICES_H */

View file

@ -22,36 +22,22 @@
#include <android-base/scopeguard.h> #include <android-base/scopeguard.h>
#include <gtest/gtest.h> #include <gtest/gtest.h>
template <char** (*Function)(uevent*)> template <std::vector<std::string> (*Function)(uevent*)>
void test_get_symlinks(const std::string& platform_device_name, uevent* uevent, void test_get_symlinks(const std::string& platform_device_name, uevent* uevent,
const std::vector<std::string> expected_links) { const std::vector<std::string> expected_links) {
add_platform_device(platform_device_name.c_str()); add_platform_device(platform_device_name.c_str());
auto platform_device_remover = android::base::make_scope_guard( auto platform_device_remover = android::base::make_scope_guard(
[&platform_device_name]() { remove_platform_device(platform_device_name.c_str()); }); [&platform_device_name]() { remove_platform_device(platform_device_name.c_str()); });
char** result = Function(uevent); std::vector<std::string> result = Function(uevent);
auto result_freer = android::base::make_scope_guard([result]() {
if (result) {
for (int i = 0; result[i]; i++) {
free(result[i]);
}
free(result);
}
});
auto expected_size = expected_links.size(); auto expected_size = expected_links.size();
if (expected_size == 0) { ASSERT_EQ(expected_size, result.size());
ASSERT_EQ(nullptr, result); if (expected_size == 0) return;
} else {
ASSERT_NE(nullptr, result);
// First assert size is equal, so we don't overrun expected_links
unsigned int size = 0;
while (result[size]) ++size;
ASSERT_EQ(expected_size, size);
for (unsigned int i = 0; i < size; ++i) { // Explicitly iterate so the results are visible if a failure occurs
EXPECT_EQ(expected_links[i], result[i]); for (unsigned int i = 0; i < expected_size; ++i) {
} EXPECT_EQ(expected_links[i], result[i]);
} }
} }
@ -208,6 +194,16 @@ TEST(devices, get_block_device_symlinks_success_pci) {
test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result); test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result);
} }
TEST(devices, get_block_device_symlinks_pci_bad_format) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
.path = "/devices/pci//mmcblk0", .partition_name = nullptr, .partition_num = -1,
};
std::vector<std::string> expected_result{};
test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result);
}
TEST(devices, get_block_device_symlinks_success_vbd) { TEST(devices, get_block_device_symlinks_success_vbd) {
const char* platform_device = "/devices/do/not/match"; const char* platform_device = "/devices/do/not/match";
uevent uevent = { uevent uevent = {
@ -218,6 +214,16 @@ TEST(devices, get_block_device_symlinks_success_vbd) {
test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result); test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result);
} }
TEST(devices, get_block_device_symlinks_vbd_bad_format) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
.path = "/devices/vbd-/mmcblk0", .partition_name = nullptr, .partition_num = -1,
};
std::vector<std::string> expected_result{};
test_get_symlinks<get_block_device_symlinks>(platform_device, &uevent, expected_result);
}
TEST(devices, get_block_device_symlinks_no_matches) { TEST(devices, get_block_device_symlinks_no_matches) {
const char* platform_device = "/devices/soc.0/f9824900.sdhci"; const char* platform_device = "/devices/soc.0/f9824900.sdhci";
uevent uevent = { uevent uevent = {
@ -236,7 +242,7 @@ TEST(devices, sanitize_null) {
TEST(devices, sanitize_empty) { TEST(devices, sanitize_empty) {
std::string empty; std::string empty;
sanitize_partition_name(&empty[0]); sanitize_partition_name(&empty);
EXPECT_EQ(0u, empty.size()); EXPECT_EQ(0u, empty.size());
} }
@ -247,24 +253,24 @@ TEST(devices, sanitize_allgood) {
"0123456789" "0123456789"
"_-."; "_-.";
std::string good_copy = good; std::string good_copy = good;
sanitize_partition_name(&good[0]); sanitize_partition_name(&good);
EXPECT_EQ(good_copy, good); EXPECT_EQ(good_copy, good);
} }
TEST(devices, sanitize_somebad) { TEST(devices, sanitize_somebad) {
std::string string = "abc!@#$%^&*()"; std::string string = "abc!@#$%^&*()";
sanitize_partition_name(&string[0]); sanitize_partition_name(&string);
EXPECT_EQ("abc__________", string); EXPECT_EQ("abc__________", string);
} }
TEST(devices, sanitize_allbad) { TEST(devices, sanitize_allbad) {
std::string string = "!@#$%^&*()"; std::string string = "!@#$%^&*()";
sanitize_partition_name(&string[0]); sanitize_partition_name(&string);
EXPECT_EQ("__________", string); EXPECT_EQ("__________", string);
} }
TEST(devices, sanitize_onebad) { TEST(devices, sanitize_onebad) {
std::string string = ")"; std::string string = ")";
sanitize_partition_name(&string[0]); sanitize_partition_name(&string);
EXPECT_EQ("_", string); EXPECT_EQ("_", string);
} }