diff --git a/init/devices.cpp b/init/devices.cpp index ad313a0e3..6e13863be 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -14,6 +14,8 @@ * limitations under the License. */ +#include "devices.h" + #include #include #include @@ -27,23 +29,19 @@ #include #include #include -#include #include #include #include #include #include -#include #include -#include #include #include #include #include #include -#include #include #include #include @@ -51,119 +49,67 @@ #include #include -#include "devices.h" #include "ueventd_parser.h" #include "util.h" -#define SYSFS_PREFIX "/sys" - extern struct selabel_handle *sehandle; static android::base::unique_fd device_fd; -struct perms_ { - char *name; - char *attr; - mode_t perm; - unsigned int uid; - unsigned int gid; - unsigned short prefix; - unsigned short wildcard; -}; - -struct perm_node { - struct perms_ dp; - struct listnode plist; -}; - -static list_declare(sys_perms); -static list_declare(dev_perms); - -int add_dev_perms(const char *name, const char *attr, - mode_t perm, unsigned int uid, unsigned int gid, - unsigned short prefix, - unsigned short wildcard) { - struct perm_node *node = (perm_node*) calloc(1, sizeof(*node)); - if (!node) - return -ENOMEM; - - node->dp.name = strdup(name); - if (!node->dp.name) { - free(node); - return -ENOMEM; +Permissions::Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid) + : name_(name), perm_(perm), uid_(uid), gid_(gid), prefix_(false), wildcard_(false) { + // If the first * is the last character, then we'll treat name_ as a prefix + // Otherwise, if a * is present, then we do a full fnmatch(). + auto wildcard_position = name_.find('*'); + if (wildcard_position == name_.length() - 1) { + prefix_ = true; + name_.pop_back(); + } else if (wildcard_position != std::string::npos) { + wildcard_ = true; } - - if (attr) { - node->dp.attr = strdup(attr); - if (!node->dp.attr) { - free(node->dp.name); - free(node); - return -ENOMEM; - } - } - - node->dp.perm = perm; - node->dp.uid = uid; - node->dp.gid = gid; - node->dp.prefix = prefix; - node->dp.wildcard = wildcard; - - if (attr) - list_add_tail(&sys_perms, &node->plist); - else - list_add_tail(&dev_perms, &node->plist); - - return 0; } -static bool perm_path_matches(const char *path, struct perms_ *dp) -{ - if (dp->prefix) { - if (strncmp(path, dp->name, strlen(dp->name)) == 0) - return true; - } else if (dp->wildcard) { - if (fnmatch(dp->name, path, FNM_PATHNAME) == 0) - return true; +bool Permissions::Match(const std::string& path) const { + if (prefix_) { + return android::base::StartsWith(path, name_.c_str()); + } else if (wildcard_) { + return fnmatch(name_.c_str(), path.c_str(), FNM_PATHNAME) == 0; } else { - if (strcmp(path, dp->name) == 0) - return true; + return path == name_; } return false; } -static bool match_subsystem(perms_* dp, const char* pattern, - const char* path, const char* subsystem) { - if (!pattern || !subsystem || strstr(dp->name, subsystem) == NULL) { - return false; +bool SysfsPermissions::MatchWithSubsystem(const std::string& path, + const std::string& subsystem) const { + std::string path_basename = android::base::Basename(path); + if (name().find(subsystem) != std::string::npos) { + if (Match("/sys/class/" + subsystem + "/" + path_basename)) return true; + if (Match("/sys/bus/" + subsystem + "/devices/" + path_basename)) return true; } - - std::string subsys_path = android::base::StringPrintf(pattern, subsystem, basename(path)); - return perm_path_matches(subsys_path.c_str(), dp); + return Match(path); } -static void fixup_sys_perms(const std::string& upath, const std::string& subsystem) { +void SysfsPermissions::SetPermissions(const std::string& path) const { + std::string attribute_file = path + "/" + attribute_; + LOG(INFO) << "fixup " << attribute_file << " " << uid() << " " << gid() << " " << std::oct + << perm(); + chown(attribute_file.c_str(), uid(), gid()); + chmod(attribute_file.c_str(), perm()); +} + +// TODO: Move these to be member variables of a future devices class. +std::vector dev_permissions; +std::vector sysfs_permissions; + +static void fixup_sys_permissions(const std::string& upath, const std::string& subsystem) { // upaths omit the "/sys" that paths in this list // contain, so we prepend it... - std::string path = SYSFS_PREFIX + upath; + std::string path = "/sys" + upath; - listnode* node; - list_for_each(node, &sys_perms) { - perms_* dp = &(node_to_item(node, perm_node, plist))->dp; - if (match_subsystem(dp, SYSFS_PREFIX "/class/%s/%s", path.c_str(), subsystem.c_str())) { - ; // matched - } else if (match_subsystem(dp, SYSFS_PREFIX "/bus/%s/devices/%s", path.c_str(), - subsystem.c_str())) { - ; // matched - } else if (!perm_path_matches(path.c_str(), dp)) { - continue; - } - - std::string attr_file = path + "/" + dp->attr; - LOG(INFO) << "fixup " << attr_file - << " " << dp->uid << " " << dp->gid << " " << std::oct << dp->perm; - chown(attr_file.c_str(), dp->uid, dp->gid); - chmod(attr_file.c_str(), dp->perm); + for (const auto& s : sysfs_permissions) { + if (s.MatchWithSubsystem(path, subsystem)) s.SetPermissions(path); } if (access(path.c_str(), F_OK) == 0) { @@ -172,42 +118,26 @@ static void fixup_sys_perms(const std::string& upath, const std::string& subsyst } } -static mode_t get_device_perm(const char* path, const std::vector& links, - unsigned* uid, unsigned* gid) { - struct listnode *node; - struct perm_node *perm_node; - struct perms_ *dp; - - /* search the perms list in reverse so that ueventd.$hardware can - * override ueventd.rc - */ - list_for_each_reverse(node, &dev_perms) { - perm_node = node_to_item(node, struct perm_node, plist); - dp = &perm_node->dp; - - if (perm_path_matches(path, dp) || - std::any_of(links.begin(), links.end(), - [dp](const auto& link) { return perm_path_matches(link.c_str(), dp); })) { - *uid = dp->uid; - *gid = dp->gid; - return dp->perm; +static std::tuple get_device_permissions( + const std::string& path, const std::vector& links) { + // Search the perms list in reverse so that ueventd.$hardware can override ueventd.rc. + for (auto it = dev_permissions.rbegin(); it != dev_permissions.rend(); ++it) { + if (it->Match(path) || std::any_of(links.begin(), links.end(), + [it](const auto& link) { return it->Match(link); })) { + return {it->perm(), it->uid(), it->gid()}; } } /* Default if nothing found. */ - *uid = 0; - *gid = 0; - return 0600; + return {0600, 0, 0}; } static void make_device(const std::string& path, int block, int major, int minor, const std::vector& links) { - unsigned uid; - unsigned gid; - mode_t mode; dev_t dev; char *secontext = NULL; - mode = get_device_perm(path.c_str(), links, &uid, &gid) | (block ? S_IFBLK : S_IFCHR); + auto [mode, uid, gid] = get_device_permissions(path, links); + mode |= (block ? S_IFBLK : S_IFCHR); if (sehandle) { std::vector c_links; @@ -607,7 +537,7 @@ static void handle_generic_device_event(uevent* uevent) { static void handle_device_event(struct uevent *uevent) { if (uevent->action == "add" || uevent->action == "change" || uevent->action == "online") { - fixup_sys_perms(uevent->path, uevent->subsystem); + fixup_sys_permissions(uevent->path, uevent->subsystem); } if (uevent->subsystem == "block") { diff --git a/init/devices.h b/init/devices.h index b8b039f0a..2cbae66a6 100644 --- a/init/devices.h +++ b/init/devices.h @@ -18,6 +18,7 @@ #define _INIT_DEVICES_H #include +#include #include #include @@ -47,15 +48,48 @@ struct uevent { int minor; }; +class Permissions { + public: + Permissions(const std::string& name, mode_t perm, uid_t uid, gid_t gid); + + bool Match(const std::string& path) const; + + mode_t perm() const { return perm_; } + uid_t uid() const { return uid_; } + gid_t gid() const { return gid_; } + + protected: + const std::string& name() const { return name_; } + + private: + std::string name_; + mode_t perm_; + uid_t uid_; + gid_t gid_; + bool prefix_; + bool wildcard_; +}; + +class SysfsPermissions : public Permissions { + public: + SysfsPermissions(const std::string& name, const std::string& attribute, mode_t perm, uid_t uid, + gid_t gid) + : Permissions(name, perm, uid, gid), attribute_(attribute) {} + + bool MatchWithSubsystem(const std::string& path, const std::string& subsystem) const; + void SetPermissions(const std::string& path) const; + + private: + const std::string attribute_; +}; + +extern std::vector dev_permissions; +extern std::vector sysfs_permissions; + typedef std::function coldboot_callback; extern coldboot_action_t handle_device_fd(coldboot_callback fn = nullptr); extern void device_init(const char* path = nullptr, coldboot_callback fn = nullptr); extern void device_close(); - -extern int add_dev_perms(const char *name, const char *attr, - mode_t perm, unsigned int uid, - unsigned int gid, unsigned short prefix, - unsigned short wildcard); int get_device_fd(); // Exposed for testing diff --git a/init/devices_test.cpp b/init/devices_test.cpp index 693fb54dd..66521db6b 100644 --- a/init/devices_test.cpp +++ b/init/devices_test.cpp @@ -316,3 +316,94 @@ TEST(devices, sanitize_onebad) { sanitize_partition_name(&string); EXPECT_EQ("_", string); } + +TEST(devices, DevPermissionsMatchNormal) { + // Basic from ueventd.rc + // /dev/null 0666 root root + Permissions permissions("/dev/null", 0666, 0, 0); + EXPECT_TRUE(permissions.Match("/dev/null")); + EXPECT_FALSE(permissions.Match("/dev/nullsuffix")); + EXPECT_FALSE(permissions.Match("/dev/nul")); + EXPECT_EQ(0666U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(0U, permissions.gid()); +} + +TEST(devices, DevPermissionsMatchPrefix) { + // Prefix from ueventd.rc + // /dev/dri/* 0666 root graphics + Permissions permissions("/dev/dri/*", 0666, 0, 1000); + EXPECT_TRUE(permissions.Match("/dev/dri/some_dri_device")); + EXPECT_TRUE(permissions.Match("/dev/dri/some_other_dri_device")); + EXPECT_TRUE(permissions.Match("/dev/dri/")); + EXPECT_FALSE(permissions.Match("/dev/dr/non_match")); + EXPECT_EQ(0666U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1000U, permissions.gid()); +} + +TEST(devices, DevPermissionsMatchWildcard) { + // Wildcard example + // /dev/device*name 0666 root graphics + Permissions permissions("/dev/device*name", 0666, 0, 1000); + EXPECT_TRUE(permissions.Match("/dev/devicename")); + EXPECT_TRUE(permissions.Match("/dev/device123name")); + EXPECT_TRUE(permissions.Match("/dev/deviceabcname")); + EXPECT_FALSE(permissions.Match("/dev/device123name/subdevice")); + EXPECT_FALSE(permissions.Match("/dev/deviceame")); + EXPECT_EQ(0666U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1000U, permissions.gid()); +} + +TEST(devices, DevPermissionsMatchWildcardPrefix) { + // Wildcard+Prefix example + // /dev/device*name* 0666 root graphics + Permissions permissions("/dev/device*name*", 0666, 0, 1000); + EXPECT_TRUE(permissions.Match("/dev/devicename")); + EXPECT_TRUE(permissions.Match("/dev/device123name")); + EXPECT_TRUE(permissions.Match("/dev/deviceabcname")); + EXPECT_TRUE(permissions.Match("/dev/device123namesomething")); + // FNM_PATHNAME doesn't match '/' with * + EXPECT_FALSE(permissions.Match("/dev/device123name/something")); + EXPECT_FALSE(permissions.Match("/dev/deviceame")); + EXPECT_EQ(0666U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1000U, permissions.gid()); +} + +TEST(devices, SysfsPermissionsMatchWithSubsystemNormal) { + // /sys/devices/virtual/input/input* enable 0660 root input + SysfsPermissions permissions("/sys/devices/virtual/input/input*", "enable", 0660, 0, 1001); + EXPECT_TRUE(permissions.MatchWithSubsystem("/sys/devices/virtual/input/input0", "input")); + EXPECT_FALSE(permissions.MatchWithSubsystem("/sys/devices/virtual/input/not_input0", "input")); + EXPECT_EQ(0660U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1001U, permissions.gid()); +} + +TEST(devices, SysfsPermissionsMatchWithSubsystemClass) { + // /sys/class/input/event* enable 0660 root input + SysfsPermissions permissions("/sys/class/input/event*", "enable", 0660, 0, 1001); + EXPECT_TRUE(permissions.MatchWithSubsystem( + "/sys/devices/soc.0/f9924000.i2c/i2c-2/2-0020/input/input0/event0", "input")); + EXPECT_FALSE(permissions.MatchWithSubsystem( + "/sys/devices/soc.0/f9924000.i2c/i2c-2/2-0020/input/input0/not_event0", "input")); + EXPECT_FALSE(permissions.MatchWithSubsystem( + "/sys/devices/soc.0/f9924000.i2c/i2c-2/2-0020/input/input0/event0", "not_input")); + EXPECT_EQ(0660U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1001U, permissions.gid()); +} + +TEST(devices, SysfsPermissionsMatchWithSubsystemBus) { + // /sys/bus/i2c/devices/i2c-* enable 0660 root input + SysfsPermissions permissions("/sys/bus/i2c/devices/i2c-*", "enable", 0660, 0, 1001); + EXPECT_TRUE(permissions.MatchWithSubsystem("/sys/devices/soc.0/f9967000.i2c/i2c-5", "i2c")); + EXPECT_FALSE(permissions.MatchWithSubsystem("/sys/devices/soc.0/f9967000.i2c/not-i2c", "i2c")); + EXPECT_FALSE( + permissions.MatchWithSubsystem("/sys/devices/soc.0/f9967000.i2c/i2c-5", "not_i2c")); + EXPECT_EQ(0660U, permissions.perm()); + EXPECT_EQ(0U, permissions.uid()); + EXPECT_EQ(1001U, permissions.gid()); +} diff --git a/init/ueventd.cpp b/init/ueventd.cpp index ea767a850..b6c6a01b1 100644 --- a/init/ueventd.cpp +++ b/init/ueventd.cpp @@ -101,8 +101,6 @@ void set_device_permission(const char* fn, int line, int nargs, char **args) mode_t perm; uid_t uid; gid_t gid; - int prefix = 0; - int wildcard = 0; char *endptr; if (nargs == 0) @@ -125,15 +123,6 @@ void set_device_permission(const char* fn, int line, int nargs, char **args) return; } - int len = strlen(name); - char *wildcard_chr = strchr(name, '*'); - if ((name[len - 1] == '*') && (wildcard_chr == (name + len - 1))) { - prefix = 1; - name[len - 1] = '\0'; - } else if (wildcard_chr) { - wildcard = 1; - } - perm = strtol(args[1], &endptr, 8); if (!endptr || *endptr != '\0') { LOG(ERROR) << "invalid mode (" << fn << ":" << line << ") '" << args[1] << "'"; @@ -154,13 +143,9 @@ void set_device_permission(const char* fn, int line, int nargs, char **args) } gid = grp->gr_gid; - if (add_dev_perms(name, attr, perm, uid, gid, prefix, wildcard) != 0) { - PLOG(ERROR) << "add_dev_perms(name=" << name << - ", attr=" << attr << - ", perm=" << std::oct << perm << std::dec << - ", uid=" << uid << ", gid=" << gid << - ", prefix=" << prefix << ", wildcard=" << wildcard << - ")"; - return; + if (attr) { + sysfs_permissions.emplace_back(name, attr, perm, uid, gid); + } else { + dev_permissions.emplace_back(name, perm, uid, gid); } }