From 780a71e779e8e41681c3f0ee12d7f559dc52b7a7 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 4 Apr 2017 16:30:40 -0700 Subject: [PATCH 1/3] ueventd: move subsystem logic from code to ueventd.rc Test: Boot bullhead Test: Boot sailfish, observe no boot time regression Test: init unit tests Change-Id: I690137b584fcc2b9cd2dd932a2678f75a56d6737 --- init/devices.cpp | 37 +++++-------------------------------- rootdir/ueventd.rc | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/init/devices.cpp b/init/devices.cpp index 52f018b98..4d6344812 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -679,7 +679,6 @@ static void mkdir_recursive_for_devpath(const char *devpath) static void handle_generic_device_event(struct uevent *uevent) { - const char *base; const char *name; char devpath[DEVPATH_LEN] = {0}; @@ -735,39 +734,13 @@ static void handle_generic_device_event(struct uevent *uevent) /* ignore other USB events */ return; } - } else if (!strncmp(uevent->subsystem, "graphics", 8)) { - base = "/dev/graphics/"; - make_dir(base, 0755); - } else if (!strncmp(uevent->subsystem, "drm", 3)) { - base = "/dev/dri/"; - make_dir(base, 0755); - } else if (!strncmp(uevent->subsystem, "oncrpc", 6)) { - base = "/dev/oncrpc/"; - make_dir(base, 0755); - } else if (!strncmp(uevent->subsystem, "adsp", 4)) { - base = "/dev/adsp/"; - make_dir(base, 0755); - } else if (!strncmp(uevent->subsystem, "msm_camera", 10)) { - base = "/dev/msm_camera/"; - make_dir(base, 0755); - } else if(!strncmp(uevent->subsystem, "input", 5)) { - base = "/dev/input/"; - make_dir(base, 0755); - } else if(!strncmp(uevent->subsystem, "mtd", 3)) { - base = "/dev/mtd/"; - make_dir(base, 0755); - } else if(!strncmp(uevent->subsystem, "sound", 5)) { - base = "/dev/snd/"; - make_dir(base, 0755); - } else - base = "/dev/"; - auto links = get_character_device_symlinks(uevent); + } else { + snprintf(devpath, sizeof(devpath), "/dev/%s", name); + } - if (!devpath[0]) - snprintf(devpath, sizeof(devpath), "%s%s", base, name); + auto links = get_character_device_symlinks(uevent); - handle_device(uevent->action, devpath, uevent->path, 0, - uevent->major, uevent->minor, links); + handle_device(uevent->action, devpath, uevent->path, 0, uevent->major, uevent->minor, links); } static void handle_device_event(struct uevent *uevent) diff --git a/rootdir/ueventd.rc b/rootdir/ueventd.rc index 579ee32d9..1609ef2db 100644 --- a/rootdir/ueventd.rc +++ b/rootdir/ueventd.rc @@ -1,5 +1,37 @@ subsystem adf - devname uevent_devname + devname uevent_devname + +subsystem graphics + devname uevent_devpath + dirname /dev/graphics + +subsystem drm + devname uevent_devpath + dirname /dev/dri + +subsystem oncrpc + devname uevent_devpath + dirname /dev/oncrpc + +subsystem adsp + devname uevent_devpath + dirname /dev/adsp + +subsystem msm_camera + devname uevent_devpath + dirname /dev/msm_camera + +subsystem input + devname uevent_devpath + dirname /dev/input + +subsystem mtd + devname uevent_devpath + dirname /dev/mtd + +subsystem sound + devname uevent_devpath + dirname /dev/snd # ueventd can only set permissions on device nodes and their associated # sysfs attributes, not on arbitrary paths. From 060b74baad7c366cb6c74042bf307f1548a8331f Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Wed, 12 Apr 2017 14:27:51 -0700 Subject: [PATCH 2/3] ueventd: convert mkdir_recursive() to std::string Bug: 36250207 Test: Boot bullhead Test: Boot sailfish, observe no boot time regression Test: init unit tests Change-Id: I5a2ac369d846e044230b709fd07eb21ad12d47bb --- init/Android.mk | 3 ++- init/util.cpp | 39 ++++++++++----------------------------- init/util.h | 2 +- init/util_test.cpp | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 31 deletions(-) diff --git a/init/Android.mk b/init/Android.mk index dbbf40a62..7e9b6136c 100644 --- a/init/Android.mk +++ b/init/Android.mk @@ -148,8 +148,9 @@ LOCAL_SRC_FILES := \ util_test.cpp \ LOCAL_SHARED_LIBRARIES += \ - libcutils \ libbase \ + libcutils \ + libselinux \ LOCAL_STATIC_LIBRARIES := libinit LOCAL_SANITIZE := integer diff --git a/init/util.cpp b/init/util.cpp index bbc59cb88..32ae93de0 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -194,37 +194,18 @@ bool write_file(const std::string& path, const std::string& content) { return success; } -int mkdir_recursive(const char *pathname, mode_t mode) -{ - char buf[128]; - const char *slash; - const char *p = pathname; - int width; - int ret; - struct stat info; - - while ((slash = strchr(p, '/')) != NULL) { - width = slash - pathname; - p = slash + 1; - if (width < 0) - break; - if (width == 0) - continue; - if ((unsigned int)width > sizeof(buf) - 1) { - LOG(ERROR) << "path too long for mkdir_recursive"; - return -1; - } - memcpy(buf, pathname, width); - buf[width] = 0; - if (stat(buf, &info) != 0) { - ret = make_dir(buf, mode); - if (ret && errno != EEXIST) - return ret; +int mkdir_recursive(const std::string& path, mode_t mode) { + std::string::size_type slash = 0; + while ((slash = path.find('/', slash + 1)) != std::string::npos) { + auto directory = path.substr(0, slash); + struct stat info; + if (stat(directory.c_str(), &info) != 0) { + auto ret = make_dir(directory.c_str(), mode); + if (ret && errno != EEXIST) return ret; } } - ret = make_dir(pathname, mode); - if (ret && errno != EEXIST) - return ret; + auto ret = make_dir(path.c_str(), mode); + if (ret && errno != EEXIST) return ret; return 0; } diff --git a/init/util.h b/init/util.h index 38a7bdb3a..0bb9cdfc4 100644 --- a/init/util.h +++ b/init/util.h @@ -60,7 +60,7 @@ std::ostream& operator<<(std::ostream& os, const Timer& t); unsigned int decode_uid(const char *s); -int mkdir_recursive(const char *pathname, mode_t mode); +int mkdir_recursive(const std::string& pathname, mode_t mode); int wait_for_file(const char *filename, std::chrono::nanoseconds timeout); void import_kernel_cmdline(bool in_qemu, const std::function&); diff --git a/init/util_test.cpp b/init/util_test.cpp index 0c0350a60..b8b409a1a 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -120,3 +120,39 @@ TEST(util, decode_uid) { EXPECT_EQ(UINT_MAX, decode_uid("toot")); EXPECT_EQ(123U, decode_uid("123")); } + +TEST(util, is_dir) { + TemporaryDir test_dir; + EXPECT_TRUE(is_dir(test_dir.path)); + TemporaryFile tf; + EXPECT_FALSE(is_dir(tf.path)); +} + +// sehandle is needed for make_dir() +// TODO: Remove once sehandle is encapsulated +#include +selabel_handle* sehandle; + +TEST(util, mkdir_recursive) { + TemporaryDir test_dir; + std::string path = android::base::StringPrintf("%s/three/directories/deep", test_dir.path); + EXPECT_EQ(0, mkdir_recursive(path, 0755)); + std::string path1 = android::base::StringPrintf("%s/three", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); + std::string path2 = android::base::StringPrintf("%s/three/directories", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); + std::string path3 = android::base::StringPrintf("%s/three/directories/deep", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); +} + +TEST(util, mkdir_recursive_extra_slashes) { + TemporaryDir test_dir; + std::string path = android::base::StringPrintf("%s/three////directories/deep//", test_dir.path); + EXPECT_EQ(0, mkdir_recursive(path, 0755)); + std::string path1 = android::base::StringPrintf("%s/three", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); + std::string path2 = android::base::StringPrintf("%s/three/directories", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); + std::string path3 = android::base::StringPrintf("%s/three/directories/deep", test_dir.path); + EXPECT_TRUE(is_dir(path1.c_str())); +} From 3fa467338fbf3462ed7617efd95bb1a048d1bdbf Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 11 Apr 2017 14:19:50 -0700 Subject: [PATCH 3/3] ueventd: Fix up string handling in handle_*_device_event() Bug: 36250207 Test: Boot bullhead Test: Boot sailfish, observe no boot time regression Test: init unit tests Change-Id: Ie5ec609a3f74bb03f5920734ada4d7de57508de4 --- init/devices.cpp | 140 +++++++++++++---------------------------------- 1 file changed, 38 insertions(+), 102 deletions(-) diff --git a/init/devices.cpp b/init/devices.cpp index 4d6344812..6cd35648a 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -604,93 +604,35 @@ static void handle_platform_device_event(struct uevent *uevent) remove_platform_device(path); } -static const char *parse_device_name(struct uevent *uevent, unsigned int len) -{ - const char *name; +static void handle_block_device_event(uevent* uevent) { + // if it's not a /dev device, nothing to do + if (uevent->major < 0 || uevent->minor < 0) return; - /* if it's not a /dev device, nothing else to do */ - if((uevent->major < 0) || (uevent->minor < 0)) - return NULL; - - /* do we have a name? */ - name = strrchr(uevent->path, '/'); - if(!name) - return NULL; - name++; - - /* too-long names would overrun our buffer */ - if(strlen(name) > len) { - LOG(ERROR) << "DEVPATH=" << name << " exceeds " << len << "-character limit on filename; ignoring event"; - return NULL; - } - - return name; -} - -#define DEVPATH_LEN 96 -#define MAX_DEV_NAME 64 - -static void handle_block_device_event(struct uevent *uevent) -{ - const char *base = "/dev/block/"; - const char *name; - char devpath[DEVPATH_LEN]; - - name = parse_device_name(uevent, MAX_DEV_NAME); - if (!name) - return; - - snprintf(devpath, sizeof(devpath), "%s%s", base, name); + const char* base = "/dev/block/"; make_dir(base, 0755); + std::string name = android::base::Basename(uevent->path); + std::string devpath = base + name; + std::vector links; if (!strncmp(uevent->path, "/devices/", 9)) links = get_block_device_symlinks(uevent); - handle_device(uevent->action, devpath, uevent->path, 1, - uevent->major, uevent->minor, links); + handle_device(uevent->action, devpath.c_str(), uevent->path, 1, uevent->major, uevent->minor, + links); } -static bool assemble_devpath(char *devpath, const char *dirname, - const char *devname) -{ - int s = snprintf(devpath, DEVPATH_LEN, "%s/%s", dirname, devname); - if (s < 0) { - PLOG(ERROR) << "failed to assemble device path; ignoring event"; - return false; - } else if (s >= DEVPATH_LEN) { - LOG(ERROR) << dirname << "/" << devname - << " exceeds " << DEVPATH_LEN << "-character limit on path; ignoring event"; - return false; - } - return true; -} +static void handle_generic_device_event(uevent* uevent) { + // if it's not a /dev device, nothing to do + if (uevent->major < 0 || uevent->minor < 0) return; -static void mkdir_recursive_for_devpath(const char *devpath) -{ - char dir[DEVPATH_LEN]; - char *slash; + std::string name = android::base::Basename(uevent->path); + ueventd_subsystem* subsystem = ueventd_subsystem_find_by_name(uevent->subsystem); - strcpy(dir, devpath); - slash = strrchr(dir, '/'); - *slash = '\0'; - mkdir_recursive(dir, 0755); -} - -static void handle_generic_device_event(struct uevent *uevent) -{ - const char *name; - char devpath[DEVPATH_LEN] = {0}; - - name = parse_device_name(uevent, MAX_DEV_NAME); - if (!name) - return; - - struct ueventd_subsystem *subsystem = - ueventd_subsystem_find_by_name(uevent->subsystem); + std::string devpath; if (subsystem) { - const char *devname; + std::string devname; switch (subsystem->devname_src) { case DEVNAME_UEVENT_DEVNAME: @@ -706,41 +648,35 @@ static void handle_generic_device_event(struct uevent *uevent) return; } - if (!assemble_devpath(devpath, subsystem->dirname, devname)) - return; - mkdir_recursive_for_devpath(devpath); + // TODO: Remove std::string() + devpath = std::string(subsystem->dirname) + "/" + devname; + mkdir_recursive(android::base::Dirname(devpath), 0755); } else if (!strncmp(uevent->subsystem, "usb", 3)) { - if (!strcmp(uevent->subsystem, "usb")) { + if (!strcmp(uevent->subsystem, "usb")) { if (uevent->device_name) { - if (!assemble_devpath(devpath, "/dev", uevent->device_name)) - return; - mkdir_recursive_for_devpath(devpath); - } - else { - /* This imitates the file system that would be created - * if we were using devfs instead. - * Minors are broken up into groups of 128, starting at "001" - */ - int bus_id = uevent->minor / 128 + 1; - int device_id = uevent->minor % 128 + 1; - /* build directories */ - make_dir("/dev/bus", 0755); - make_dir("/dev/bus/usb", 0755); - snprintf(devpath, sizeof(devpath), "/dev/bus/usb/%03d", bus_id); - make_dir(devpath, 0755); - snprintf(devpath, sizeof(devpath), "/dev/bus/usb/%03d/%03d", bus_id, device_id); - } - } else { - /* ignore other USB events */ - return; - } + // TODO: Remove std::string + devpath = "/dev/" + std::string(uevent->device_name); + } else { + // This imitates the file system that would be created + // if we were using devfs instead. + // Minors are broken up into groups of 128, starting at "001" + int bus_id = uevent->minor / 128 + 1; + int device_id = uevent->minor % 128 + 1; + devpath = android::base::StringPrintf("/dev/bus/usb/%03d/%03d", bus_id, device_id); + } + mkdir_recursive(android::base::Dirname(devpath), 0755); + } else { + // ignore other USB events + return; + } } else { - snprintf(devpath, sizeof(devpath), "/dev/%s", name); + devpath = "/dev/" + name; } auto links = get_character_device_symlinks(uevent); - handle_device(uevent->action, devpath, uevent->path, 0, uevent->major, uevent->minor, links); + handle_device(uevent->action, devpath.c_str(), uevent->path, 0, uevent->major, uevent->minor, + links); } static void handle_device_event(struct uevent *uevent)