You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by al...@apache.org on 2017/01/05 15:45:50 UTC
mesos git commit: Made `getpwnam_r` error handling more robust.
Repository: mesos
Updated Branches:
refs/heads/master f89f28724 -> b2d36ed69
Made `getpwnam_r` error handling more robust.
According to POSIX, `getpwnam_r` returns 0 (and sets `result` to the
null pointer) when the specified user name is not found. However,
certain versions of Linux (e.g., RHEL7, recent Arch Linux) return
non-zero and set `errno` (to one of several different values) when
`getpwnam_r` is passed an invalid user name.
In stout, we want to treat the "invalid user name" and "user name not
found" cases the same. Both the POSIX spec and Linux manpages call out
certain errno values as definitely indicating errors (e.g., EIO,
EMFILE). Hence, we check `errno` and return an error to the caller if
`errno` appears in that list. We treat `errno` values not in that list
as equivalent to "user not found".
Review: https://reviews.apache.org/r/54952/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b2d36ed6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b2d36ed6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b2d36ed6
Branch: refs/heads/master
Commit: b2d36ed6965a2049a8877e253d5d8842f225231d
Parents: f89f287
Author: Neil Conway <ne...@gmail.com>
Authored: Thu Jan 5 16:15:45 2017 +0100
Committer: Alexander Rukletsov <al...@apache.org>
Committed: Thu Jan 5 16:15:45 2017 +0100
----------------------------------------------------------------------
3rdparty/stout/include/stout/os/posix/su.hpp | 127 +++++++++++++---------
3rdparty/stout/tests/os_tests.cpp | 7 ++
2 files changed, 81 insertions(+), 53 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/b2d36ed6/3rdparty/stout/include/stout/os/posix/su.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/posix/su.hpp b/3rdparty/stout/include/stout/os/posix/su.hpp
index f3f45eb..cb1478b 100644
--- a/3rdparty/stout/include/stout/os/posix/su.hpp
+++ b/3rdparty/stout/include/stout/os/posix/su.hpp
@@ -44,9 +44,6 @@ inline Result<uid_t> getuid(const Option<std::string>& user = None())
return ::getuid();
}
- struct passwd passwd;
- struct passwd* result = nullptr;
-
int size = sysconf(_SC_GETPW_R_SIZE_MAX);
if (size == -1) {
// Initial value for buffer size.
@@ -54,39 +51,53 @@ inline Result<uid_t> getuid(const Option<std::string>& user = None())
}
while (true) {
+ struct passwd pwd;
+ struct passwd* result;
char* buffer = new char[size];
- if (getpwnam_r(user.get().c_str(), &passwd, buffer, size, &result) == 0) {
- // The usual interpretation of POSIX is that getpwnam_r will
- // return 0 but set result == nullptr if the user is not found.
+ if (getpwnam_r(user->c_str(), &pwd, buffer, size, &result) == 0) {
+ // Per POSIX, if the user name is not found, `getpwnam_r` returns
+ // zero and sets `result` to the null pointer. (Linux behaves
+ // differently for invalid user names; see below).
if (result == nullptr) {
delete[] buffer;
return None();
}
- uid_t uid = passwd.pw_uid;
+ // Entry found.
+ uid_t uid = pwd.pw_uid;
delete[] buffer;
return uid;
} else {
- // RHEL7 (and possibly other systems) will return non-zero and
- // set one of the following errors for "The given name or uid
- // was not found." See 'man getpwnam_r'. We only check for the
- // errors explicitly listed, and do not consider the ellipsis.
- if (errno == ENOENT ||
- errno == ESRCH ||
- errno == EBADF ||
- errno == EPERM) {
- delete[] buffer;
- return None();
+ delete[] buffer;
+
+ if (errno == ERANGE) {
+ // Buffer too small; enlarge it and retry.
+ size *= 2;
+ continue;
}
- if (errno != ERANGE) {
- delete[] buffer;
- return ErrnoError("Failed to get username information");
+ // According to POSIX, a non-zero return value from `getpwnam_r`
+ // indicates an error. However, some versions of glibc return
+ // non-zero and set errno to ENOENT, ESRCH, EBADF, EPERM,
+ // EINVAL, or other values if the user name was invalid and/or
+ // not found. POSIX and Linux manpages also list certain errno
+ // values (e.g., EIO, EMFILE) as definitely indicating an error.
+ //
+ // Hence, we check for those specific error values and return an
+ // error to the caller; for any errno value not in that list, we
+ // assume the user name wasn't found.
+ //
+ // TODO(neilc): Consider retrying on EINTR.
+ if (errno != EIO &&
+ errno != EINTR &&
+ errno != EMFILE &&
+ errno != ENFILE &&
+ errno != ENOMEM) {
+ return None();
}
- // getpwnam_r set ERANGE so try again with a larger buffer.
- size *= 2;
- delete[] buffer;
+
+ return ErrnoError("Failed to get username information");
}
}
@@ -110,9 +121,6 @@ inline Result<gid_t> getgid(const Option<std::string>& user = None())
return ::getgid();
}
- struct passwd passwd;
- struct passwd* result = nullptr;
-
int size = sysconf(_SC_GETPW_R_SIZE_MAX);
if (size == -1) {
// Initial value for buffer size.
@@ -120,39 +128,53 @@ inline Result<gid_t> getgid(const Option<std::string>& user = None())
}
while (true) {
+ struct passwd pwd;
+ struct passwd* result;
char* buffer = new char[size];
- if (getpwnam_r(user.get().c_str(), &passwd, buffer, size, &result) == 0) {
- // The usual interpretation of POSIX is that getpwnam_r will
- // return 0 but set result == nullptr if the group is not found.
+ if (getpwnam_r(user->c_str(), &pwd, buffer, size, &result) == 0) {
+ // Per POSIX, if the user name is not found, `getpwnam_r` returns
+ // zero and sets `result` to the null pointer. (Linux behaves
+ // differently for invalid user names; see below).
if (result == nullptr) {
delete[] buffer;
return None();
}
- gid_t gid = passwd.pw_gid;
+ // Entry found.
+ gid_t gid = pwd.pw_gid;
delete[] buffer;
return gid;
} else {
- // RHEL7 (and possibly other systems) will return non-zero and
- // set one of the following errors for "The given name or uid
- // was not found." See 'man getpwnam_r'. We only check for the
- // errors explicitly listed, and do not consider the ellipsis.
- if (errno == ENOENT ||
- errno == ESRCH ||
- errno == EBADF ||
- errno == EPERM) {
- delete[] buffer;
- return None();
+ delete[] buffer;
+
+ if (errno == ERANGE) {
+ // Buffer too small; enlarge it and retry.
+ size *= 2;
+ continue;
}
- if (errno != ERANGE) {
- delete[] buffer;
- return ErrnoError("Failed to get username information");
+ // According to POSIX, a non-zero return value from `getpwnam_r`
+ // indicates an error. However, some versions of glibc return
+ // non-zero and set errno to ENOENT, ESRCH, EBADF, EPERM,
+ // EINVAL, or other values if the user name was invalid and/or
+ // not found. POSIX and Linux manpages also list certain errno
+ // values (e.g., EIO, EMFILE) as definitely indicating an error.
+ //
+ // Hence, we check for those specific error values and return an
+ // error to the caller; for any errno value not in that list, we
+ // assume the user name wasn't found.
+ //
+ // TODO(neilc): Consider retrying on EINTR.
+ if (errno != EIO &&
+ errno != EINTR &&
+ errno != EMFILE &&
+ errno != ENFILE &&
+ errno != ENOMEM) {
+ return None();
}
- // getpwnam_r set ERANGE so try again with a larger buffer.
- size *= 2;
- delete[] buffer;
+
+ return ErrnoError("Failed to get username information");
}
}
@@ -264,13 +286,12 @@ inline Result<std::string> user(Option<uid_t> uid = None())
size = 1024;
}
- struct passwd passwd;
- struct passwd* result = nullptr;
-
while (true) {
+ struct passwd pwd;
+ struct passwd* result;
char* buffer = new char[size];
- if (getpwuid_r(uid.get(), &passwd, buffer, size, &result) == 0) {
+ if (getpwuid_r(uid.get(), &pwd, buffer, size, &result) == 0) {
// getpwuid_r will return 0 but set result == nullptr if the uid is
// not found.
if (result == nullptr) {
@@ -278,18 +299,18 @@ inline Result<std::string> user(Option<uid_t> uid = None())
return None();
}
- std::string user(passwd.pw_name);
+ std::string user(pwd.pw_name);
delete[] buffer;
return user;
} else {
+ delete[] buffer;
+
if (errno != ERANGE) {
- delete[] buffer;
return ErrnoError();
}
// getpwuid_r set ERANGE so try again with a larger buffer.
size *= 2;
- delete[] buffer;
}
}
}
http://git-wip-us.apache.org/repos/asf/mesos/blob/b2d36ed6/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp
index 8d2005b..30735e2 100644
--- a/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/stout/tests/os_tests.cpp
@@ -680,9 +680,16 @@ TEST_F(OsTest, User)
ASSERT_SOME(gid);
EXPECT_SOME_EQ(gid.get(), os::getgid(user.get()));
+ // A random UUID is an invalid username on some platforms. Some
+ // versions of Linux (e.g., RHEL7) treat invalid usernames
+ // differently from valid-but-not-found usernames.
EXPECT_NONE(os::getuid(UUID::random().toString()));
EXPECT_NONE(os::getgid(UUID::random().toString()));
+ // A username that is valid but that is unlikely to exist.
+ EXPECT_NONE(os::getuid("zzzvaliduserzzz"));
+ EXPECT_NONE(os::getgid("zzzvaliduserzzz"));
+
EXPECT_SOME(os::su(user.get()));
EXPECT_ERROR(os::su(UUID::random().toString()));