init: Check DecodeUid() result and use error string
Check the result of DecodeUid() and return failure when uids/gids are
unable to be decoded.
Also, use an error string instead of logging directly such that more
context can be added when decoding fails.
Bug: 38038887
Test: Boot bullhead
Test: Init unit tests
Change-Id: I84c11aa5a8041bf5d2f754ee9af748344b789b37
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 3dadfd7..848dfdb 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -215,11 +215,19 @@
}
if (args.size() >= 4) {
- uid_t uid = decode_uid(args[3].c_str());
+ uid_t uid;
+ std::string decode_uid_err;
+ if (!DecodeUid(args[3], &uid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find UID for '" << args[3] << "': " << decode_uid_err;
+ return -1;
+ }
gid_t gid = -1;
if (args.size() == 5) {
- gid = decode_uid(args[4].c_str());
+ if (!DecodeUid(args[4], &gid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err;
+ return -1;
+ }
}
if (lchown(args[1].c_str(), uid, gid) == -1) {
@@ -655,17 +663,26 @@
}
static int do_chown(const std::vector<std::string>& args) {
- /* GID is optional. */
- if (args.size() == 3) {
- if (lchown(args[2].c_str(), decode_uid(args[1].c_str()), -1) == -1)
- return -errno;
- } else if (args.size() == 4) {
- if (lchown(args[3].c_str(), decode_uid(args[1].c_str()),
- decode_uid(args[2].c_str())) == -1)
- return -errno;
- } else {
+ uid_t uid;
+ std::string decode_uid_err;
+ if (!DecodeUid(args[1], &uid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find UID for '" << args[1] << "': " << decode_uid_err;
return -1;
}
+
+ // GID is optional and pushes the index of path out by one if specified.
+ const std::string& path = (args.size() == 4) ? args[3] : args[2];
+ gid_t gid = -1;
+
+ if (args.size() == 4) {
+ if (!DecodeUid(args[2], &gid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find GID for '" << args[2] << "': " << decode_uid_err;
+ return -1;
+ }
+ }
+
+ if (lchown(path.c_str(), uid, gid) == -1) return -errno;
+
return 0;
}
diff --git a/init/init_parser_test.cpp b/init/init_parser_test.cpp
index d8fd2ba..86d60d0 100644
--- a/init/init_parser_test.cpp
+++ b/init/init_parser_test.cpp
@@ -86,19 +86,29 @@
ASSERT_EQ("", svc->seclabel());
}
if (uid) {
- ASSERT_EQ(decode_uid("log"), svc->uid());
+ uid_t decoded_uid;
+ std::string err;
+ ASSERT_TRUE(DecodeUid("log", &decoded_uid, &err));
+ ASSERT_EQ(decoded_uid, svc->uid());
} else {
ASSERT_EQ(0U, svc->uid());
}
if (gid) {
- ASSERT_EQ(decode_uid("shell"), svc->gid());
+ uid_t decoded_uid;
+ std::string err;
+ ASSERT_TRUE(DecodeUid("shell", &decoded_uid, &err));
+ ASSERT_EQ(decoded_uid, svc->gid());
} else {
ASSERT_EQ(0U, svc->gid());
}
if (supplementary_gids) {
ASSERT_EQ(2U, svc->supp_gids().size());
- ASSERT_EQ(decode_uid("system"), svc->supp_gids()[0]);
- ASSERT_EQ(decode_uid("adb"), svc->supp_gids()[1]);
+ uid_t decoded_uid;
+ std::string err;
+ ASSERT_TRUE(DecodeUid("system", &decoded_uid, &err));
+ ASSERT_EQ(decoded_uid, svc->supp_gids()[0]);
+ ASSERT_TRUE(DecodeUid("adb", &decoded_uid, &err));
+ ASSERT_EQ(decoded_uid, svc->supp_gids()[1]);
} else {
ASSERT_EQ(0U, svc->supp_gids().size());
}
diff --git a/init/service.cpp b/init/service.cpp
index 3a9f622..4d9edc4 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -380,9 +380,18 @@
}
bool Service::ParseGroup(const std::vector<std::string>& args, std::string* err) {
- gid_ = decode_uid(args[1].c_str());
+ std::string decode_uid_err;
+ if (!DecodeUid(args[1], &gid_, &decode_uid_err)) {
+ *err = "Unable to find GID for '" + args[1] + "': " + decode_uid_err;
+ return false;
+ }
for (std::size_t n = 2; n < args.size(); n++) {
- supp_gids_.emplace_back(decode_uid(args[n].c_str()));
+ gid_t gid;
+ if (!DecodeUid(args[n], &gid, &decode_uid_err)) {
+ *err = "Unable to find GID for '" + args[n] + "': " + decode_uid_err;
+ return false;
+ }
+ supp_gids_.emplace_back(gid);
}
return true;
}
@@ -480,10 +489,25 @@
template <typename T>
bool Service::AddDescriptor(const std::vector<std::string>& args, std::string* err) {
int perm = args.size() > 3 ? std::strtoul(args[3].c_str(), 0, 8) : -1;
- uid_t uid = args.size() > 4 ? decode_uid(args[4].c_str()) : 0;
- gid_t gid = args.size() > 5 ? decode_uid(args[5].c_str()) : 0;
+ uid_t uid = 0;
+ gid_t gid = 0;
std::string context = args.size() > 6 ? args[6] : "";
+ std::string decode_uid_err;
+ if (args.size() > 4) {
+ if (!DecodeUid(args[4], &uid, &decode_uid_err)) {
+ *err = "Unable to find UID for '" + args[4] + "': " + decode_uid_err;
+ return false;
+ }
+ }
+
+ if (args.size() > 5) {
+ if (!DecodeUid(args[5], &gid, &decode_uid_err)) {
+ *err = "Unable to find GID for '" + args[5] + "': " + decode_uid_err;
+ return false;
+ }
+ }
+
auto descriptor = std::make_unique<T>(args[1], args[2], uid, gid, perm, context);
auto old =
@@ -522,7 +546,11 @@
}
bool Service::ParseUser(const std::vector<std::string>& args, std::string* err) {
- uid_ = decode_uid(args[1].c_str());
+ std::string decode_uid_err;
+ if (!DecodeUid(args[1], &uid_, &decode_uid_err)) {
+ *err = "Unable to find UID for '" + args[1] + "': " + decode_uid_err;
+ return false;
+ }
return true;
}
@@ -936,15 +964,28 @@
}
uid_t uid = 0;
if (command_arg > 3) {
- uid = decode_uid(args[2].c_str());
+ std::string decode_uid_err;
+ if (!DecodeUid(args[2], &uid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find UID for '" << args[2] << "': " << decode_uid_err;
+ return nullptr;
+ }
}
gid_t gid = 0;
std::vector<gid_t> supp_gids;
if (command_arg > 4) {
- gid = decode_uid(args[3].c_str());
+ std::string decode_uid_err;
+ if (!DecodeUid(args[3], &gid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find GID for '" << args[3] << "': " << decode_uid_err;
+ return nullptr;
+ }
std::size_t nr_supp_gids = command_arg - 1 /* -- */ - 4 /* exec SECLABEL UID GID */;
for (size_t i = 0; i < nr_supp_gids; ++i) {
- supp_gids.push_back(decode_uid(args[4 + i].c_str()));
+ gid_t supp_gid;
+ if (!DecodeUid(args[4 + i], &supp_gid, &decode_uid_err)) {
+ LOG(ERROR) << "Unable to find UID for '" << args[4 + i] << "': " << decode_uid_err;
+ return nullptr;
+ }
+ supp_gids.push_back(supp_gid);
}
}
diff --git a/init/util.cpp b/init/util.cpp
index 87b8d9d..4df0c25 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -48,39 +48,33 @@
#endif
using android::base::boot_clock;
+using namespace std::literals::string_literals;
-static unsigned int do_decode_uid(const char *s)
-{
- unsigned int v;
+// DecodeUid() - decodes and returns the given string, which can be either the
+// numeric or name representation, into the integer uid or gid. Returns
+// UINT_MAX on error.
+bool DecodeUid(const std::string& name, uid_t* uid, std::string* err) {
+ *uid = UINT_MAX;
+ *err = "";
- if (!s || *s == '\0')
- return UINT_MAX;
-
- if (isalpha(s[0])) {
- struct passwd* pwd = getpwnam(s);
- if (!pwd)
- return UINT_MAX;
- return pwd->pw_uid;
+ if (isalpha(name[0])) {
+ passwd* pwd = getpwnam(name.c_str());
+ if (!pwd) {
+ *err = "getpwnam failed: "s + strerror(errno);
+ return false;
+ }
+ *uid = pwd->pw_uid;
+ return true;
}
errno = 0;
- v = (unsigned int) strtoul(s, 0, 0);
- if (errno)
- return UINT_MAX;
- return v;
-}
-
-/*
- * decode_uid - decodes and returns the given string, which can be either the
- * numeric or name representation, into the integer uid or gid. Returns
- * UINT_MAX on error.
- */
-unsigned int decode_uid(const char *s) {
- unsigned int v = do_decode_uid(s);
- if (v == UINT_MAX) {
- LOG(ERROR) << "decode_uid: Unable to find UID for '" << s << "'; returning UINT_MAX";
+ uid_t result = static_cast<uid_t>(strtoul(name.c_str(), 0, 0));
+ if (errno) {
+ *err = "strtoul failed: "s + strerror(errno);
+ return false;
}
- return v;
+ *uid = result;
+ return true;
}
/*
diff --git a/init/util.h b/init/util.h
index 55ebded..4957f15 100644
--- a/init/util.h
+++ b/init/util.h
@@ -61,7 +61,7 @@
std::ostream& operator<<(std::ostream& os, const Timer& t);
-unsigned int decode_uid(const char *s);
+bool DecodeUid(const std::string& name, uid_t* uid, std::string* err);
int mkdir_recursive(const std::string& pathname, mode_t mode, selabel_handle* sehandle);
int wait_for_file(const char *filename, std::chrono::nanoseconds timeout);
diff --git a/init/util_test.cpp b/init/util_test.cpp
index ac23a32..a24185b 100644
--- a/init/util_test.cpp
+++ b/init/util_test.cpp
@@ -115,10 +115,21 @@
EXPECT_STREQ("2ll2", s2.c_str());
}
-TEST(util, decode_uid) {
- EXPECT_EQ(0U, decode_uid("root"));
- EXPECT_EQ(UINT_MAX, decode_uid("toot"));
- EXPECT_EQ(123U, decode_uid("123"));
+TEST(util, DecodeUid) {
+ uid_t decoded_uid;
+ std::string err;
+
+ EXPECT_TRUE(DecodeUid("root", &decoded_uid, &err));
+ EXPECT_EQ("", err);
+ EXPECT_EQ(0U, decoded_uid);
+
+ EXPECT_FALSE(DecodeUid("toot", &decoded_uid, &err));
+ EXPECT_EQ("getpwnam failed: No such file or directory", err);
+ EXPECT_EQ(UINT_MAX, decoded_uid);
+
+ EXPECT_TRUE(DecodeUid("123", &decoded_uid, &err));
+ EXPECT_EQ("", err);
+ EXPECT_EQ(123U, decoded_uid);
}
TEST(util, is_dir) {