From e7656b7200d934d6f77df422d6d3dee469230dea Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Mon, 1 May 2017 17:10:09 -0700 Subject: [PATCH] ueventd: do not reference init's sehandle Init exposes a global 'sehandle' that ueventd references as part of devices.cpp and util.cpp. This is particularly dangerous in device_init() in which both uevent and init write to this global. This change creates a separate local copy for devices.cpp and puts restrictions on where init.h can be included to make sure the global used by init is not reference by non-init code. Future changes to init should remove this global. Test: Boot bullhead Change-Id: Ifefa9e1932e9d647d06cca2618f5c8e5a7a85460 --- init/builtins.cpp | 4 ++-- init/descriptors.cpp | 2 +- init/devices.cpp | 12 ++++++++---- init/init.h | 3 +++ init/property_service.cpp | 2 +- init/util.cpp | 20 ++++++++++---------- init/util.h | 9 +++++---- init/util_test.cpp | 9 ++------- init/watchdogd.cpp | 5 ++++- 9 files changed, 36 insertions(+), 30 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 1d98ef13d..559fb8468 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -205,7 +205,7 @@ static int do_mkdir(const std::vector& args) { mode = std::strtoul(args[2].c_str(), 0, 8); } - ret = make_dir(args[1].c_str(), mode); + ret = make_dir(args[1].c_str(), mode, sehandle); /* chmod in case the directory already exists */ if (ret == -1 && errno == EEXIST) { ret = fchmodat(AT_FDCWD, args[1].c_str(), mode, AT_SYMLINK_NOFOLLOW); @@ -809,7 +809,7 @@ static int do_wait_for_prop(const std::vector& args) { * Callback to make a directory from the ext4 code */ static int do_installkeys_ensure_dir_exists(const char* dir) { - if (make_dir(dir, 0700) && errno != EEXIST) { + if (make_dir(dir, 0700, sehandle) && errno != EEXIST) { return -1; } diff --git a/init/descriptors.cpp b/init/descriptors.cpp index bc6bc8dca..b2142ca99 100644 --- a/init/descriptors.cpp +++ b/init/descriptors.cpp @@ -80,7 +80,7 @@ int SocketInfo::Create(const std::string& context) const { int flags = ((type() == "stream" ? SOCK_STREAM : (type() == "dgram" ? SOCK_DGRAM : SOCK_SEQPACKET))); - return create_socket(name().c_str(), flags, perm(), uid(), gid(), context.c_str()); + return create_socket(name().c_str(), flags, perm(), uid(), gid(), context.c_str(), sehandle); } const std::string SocketInfo::key() const { diff --git a/init/devices.cpp b/init/devices.cpp index 11687f06c..74f099ade 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -54,7 +54,11 @@ #include "ueventd.h" #include "util.h" -extern struct selabel_handle *sehandle; +#ifdef _INIT_INIT_H +#error "Do not include init.h in files used by ueventd or watchdogd; it will expose init's globals" +#endif + +static selabel_handle* sehandle; static android::base::unique_fd device_fd; @@ -554,7 +558,7 @@ std::vector get_block_device_symlinks(uevent* uevent) { } static void make_link_init(const std::string& oldpath, const std::string& newpath) { - if (mkdir_recursive(dirname(newpath.c_str()), 0755)) { + if (mkdir_recursive(dirname(newpath.c_str()), 0755, sehandle)) { PLOG(ERROR) << "Failed to create directory " << dirname(newpath.c_str()); } @@ -599,7 +603,7 @@ static void handle_block_device_event(uevent* uevent) { if (uevent->major < 0 || uevent->minor < 0) return; const char* base = "/dev/block/"; - make_dir(base, 0755); + make_dir(base, 0755, sehandle); std::string name = android::base::Basename(uevent->path); std::string devpath = base + name; @@ -641,7 +645,7 @@ static void handle_generic_device_event(uevent* uevent) { devpath = "/dev/" + android::base::Basename(uevent->path); } - mkdir_recursive(android::base::Dirname(devpath), 0755); + mkdir_recursive(android::base::Dirname(devpath), 0755, sehandle); auto links = get_character_device_symlinks(uevent); diff --git a/init/init.h b/init/init.h index 6add75fba..6725a7052 100644 --- a/init/init.h +++ b/init/init.h @@ -19,6 +19,9 @@ #include +// Note: These globals are *only* valid in init, so they should not be used in ueventd, +// watchdogd, or any files that may be included in those, such as devices.cpp and util.cpp. +// TODO: Have an Init class and remove all globals. extern const char *ENV[32]; extern std::string default_console; extern struct selabel_handle *sehandle; diff --git a/init/property_service.cpp b/init/property_service.cpp index aa47976f5..6f40d1ce0 100644 --- a/init/property_service.cpp +++ b/init/property_service.cpp @@ -659,7 +659,7 @@ void start_property_service() { property_set("ro.property_service.version", "2"); property_set_fd = create_socket(PROP_SERVICE_NAME, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK, - 0666, 0, 0, NULL); + 0666, 0, 0, nullptr, sehandle); if (property_set_fd == -1) { PLOG(ERROR) << "start_property_service socket creation failed"; exit(1); diff --git a/init/util.cpp b/init/util.cpp index a101ce520..87b8d9d81 100644 --- a/init/util.cpp +++ b/init/util.cpp @@ -40,11 +40,13 @@ #include #include #include -#include -#include "init.h" #include "reboot.h" +#ifdef _INIT_INIT_H +#error "Do not include init.h in files used by ueventd or watchdogd; it will expose init's globals" +#endif + using android::base::boot_clock; static unsigned int do_decode_uid(const char *s) @@ -87,9 +89,8 @@ unsigned int decode_uid(const char *s) { * daemon. We communicate the file descriptor's value via the environment * variable ANDROID_SOCKET_ENV_PREFIX ("ANDROID_SOCKET_foo"). */ -int create_socket(const char *name, int type, mode_t perm, uid_t uid, - gid_t gid, const char *socketcon) -{ +int create_socket(const char* name, int type, mode_t perm, uid_t uid, gid_t gid, + const char* socketcon, selabel_handle* sehandle) { if (socketcon) { if (setsockcreatecon(socketcon) == -1) { PLOG(ERROR) << "setsockcreatecon(\"" << socketcon << "\") failed"; @@ -194,17 +195,17 @@ bool write_file(const std::string& path, const std::string& content) { return success; } -int mkdir_recursive(const std::string& path, mode_t mode) { +int mkdir_recursive(const std::string& path, mode_t mode, selabel_handle* sehandle) { 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); + auto ret = make_dir(directory.c_str(), mode, sehandle); if (ret && errno != EEXIST) return ret; } } - auto ret = make_dir(path.c_str(), mode); + auto ret = make_dir(path.c_str(), mode, sehandle); if (ret && errno != EEXIST) return ret; return 0; } @@ -233,8 +234,7 @@ void import_kernel_cmdline(bool in_qemu, } } -int make_dir(const char *path, mode_t mode) -{ +int make_dir(const char* path, mode_t mode, selabel_handle* sehandle) { int rc; char *secontext = NULL; diff --git a/init/util.h b/init/util.h index 92b3a1d9e..55ebded57 100644 --- a/init/util.h +++ b/init/util.h @@ -26,6 +26,7 @@ #include #include +#include #define COLDBOOT_DONE "/dev/.coldboot_done" @@ -34,8 +35,8 @@ const std::string kAndroidDtDir("/proc/device-tree/firmware/android/"); using android::base::boot_clock; using namespace std::chrono_literals; -int create_socket(const char *name, int type, mode_t perm, - uid_t uid, gid_t gid, const char *socketcon); +int create_socket(const char* name, int type, mode_t perm, uid_t uid, gid_t gid, + const char* socketcon, selabel_handle* sehandle); bool read_file(const std::string& path, std::string* content); bool write_file(const std::string& path, const std::string& content); @@ -62,11 +63,11 @@ std::ostream& operator<<(std::ostream& os, const Timer& t); unsigned int decode_uid(const char *s); -int mkdir_recursive(const std::string& pathname, mode_t mode); +int mkdir_recursive(const std::string& pathname, mode_t mode, selabel_handle* sehandle); int wait_for_file(const char *filename, std::chrono::nanoseconds timeout); void import_kernel_cmdline(bool in_qemu, const std::function&); -int make_dir(const char *path, mode_t mode); +int make_dir(const char* path, mode_t mode, selabel_handle* sehandle); int restorecon(const char *pathname, int flags = 0); std::string bytes_to_hex(const uint8_t *bytes, size_t bytes_len); bool is_dir(const char* pathname); diff --git a/init/util_test.cpp b/init/util_test.cpp index b8b409a1a..ac23a326e 100644 --- a/init/util_test.cpp +++ b/init/util_test.cpp @@ -128,15 +128,10 @@ TEST(util, is_dir) { 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)); + EXPECT_EQ(0, mkdir_recursive(path, 0755, nullptr)); 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); @@ -148,7 +143,7 @@ TEST(util, mkdir_recursive) { 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)); + EXPECT_EQ(0, mkdir_recursive(path, 0755, nullptr)); 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); diff --git a/init/watchdogd.cpp b/init/watchdogd.cpp index 21c1e5b43..7baa48719 100644 --- a/init/watchdogd.cpp +++ b/init/watchdogd.cpp @@ -24,7 +24,10 @@ #include #include "log.h" -#include "util.h" + +#ifdef _INIT_INIT_H +#error "Do not include init.h in files used by ueventd or watchdogd; it will expose init's globals" +#endif #define DEV_NAME "/dev/watchdog"