Author: kihwal
Date: Mon Apr 21 18:27:16 2014
New Revision: 1588949
URL: http://svn.apache.org/r1588949
Log:
HADOOP-10522. JniBasedUnixGroupMapping mishandles errors. Contributed by Kihwal
Lee.
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c
Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1588949&r1=1588948&r2=1588949&view=diff
==============================================================================
--- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt
(original)
+++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Mon Apr
21 18:27:16 2014
@@ -423,6 +423,8 @@ Release 2.4.1 - UNRELEASED
HADOOP-10490. TestMapFile and TestBloomMapFile leak file descriptors.
(cnauroth)
+ HADOOP-10522. JniBasedUnixGroupMapping mishandles errors. (kihwal)
+
Release 2.4.0 - 2014-04-07
INCOMPATIBLE CHANGES
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c?rev=1588949&r1=1588948&r2=1588949&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c
(original)
+++
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c
Mon Apr 21 18:27:16 2014
@@ -120,10 +120,18 @@ Java_org_apache_hadoop_security_JniBased
goto done;
}
ret = hadoop_user_info_fetch(uinfo, username);
- if (ret == ENOENT) {
- jgroups = (*env)->NewObjectArray(env, 0, g_string_clazz, NULL);
+ if (ret) {
+ if (ret == ENOENT) {
+ jgroups = (*env)->NewObjectArray(env, 0, g_string_clazz, NULL);
+ } else { // handle other errors
+ char buf[128];
+ snprintf(buf, sizeof(buf), "getgrouplist: error looking up user. %d
(%s)",
+ ret, terror(ret));
+ THROW(env, "java/lang/RuntimeException", buf);
+ }
goto done;
}
+
ginfo = hadoop_group_info_alloc();
if (!ginfo) {
THROW(env, "java/lang/OutOfMemoryError", NULL);
@@ -135,7 +143,7 @@ Java_org_apache_hadoop_security_JniBased
THROW(env, "java/lang/OutOfMemoryError", NULL);
} else {
char buf[128];
- snprintf(buf, sizeof(buf), "getgrouplist error %d (%s)",
+ snprintf(buf, sizeof(buf), "getgrouplist: error looking up groups. %d
(%s)",
ret, terror(ret));
THROW(env, "java/lang/RuntimeException", buf);
}
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c?rev=1588949&r1=1588948&r2=1588949&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c
(original)
+++
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c
Mon Apr 21 18:27:16 2014
@@ -27,6 +27,8 @@
#include <string.h>
#include <unistd.h>
+#define MAX_GROUP_LOOKUP_TRIES 5
+
struct hadoop_group_info *hadoop_group_info_alloc(void)
{
struct hadoop_group_info *ginfo;
@@ -84,31 +86,48 @@ static int getgrgid_error_translate(int
int hadoop_group_info_fetch(struct hadoop_group_info *ginfo, gid_t gid)
{
struct group *group;
- int err;
+ int ret, i;
size_t buf_sz;
char *nbuf;
hadoop_group_info_clear(ginfo);
- for (;;) {
- do {
- group = NULL;
- err = getgrgid_r(gid, &ginfo->group, ginfo->buf,
- ginfo->buf_sz, &group);
- } while ((!group) && (err == EINTR));
- if (group) {
- return 0;
- }
- if (err != ERANGE) {
- return getgrgid_error_translate(errno);
+ for (i = 0, ret = 0; i < MAX_GROUP_LOOKUP_TRIES; i++) {
+ // If the previous call returned ERANGE, increase the buffer size
+ if (ret == ERANGE) {
+ buf_sz = ginfo->buf_sz * 2;
+ nbuf = realloc(ginfo->buf, buf_sz);
+ if (!nbuf) {
+ return ENOMEM;
+ }
+ ginfo->buf = nbuf;
+ ginfo->buf_sz = buf_sz;
}
- buf_sz = ginfo->buf_sz * 2;
- nbuf = realloc(ginfo->buf, buf_sz);
- if (!nbuf) {
- return ENOMEM;
+
+ // The following call returns errno. Reading the global errno wihtout
+ // locking is not thread-safe.
+ group = NULL;
+ ret = getgrgid_r(gid, &ginfo->group, ginfo->buf,
+ ginfo->buf_sz, &group);
+ switch(ret) {
+ case 0:
+ if (!group) {
+ // The underlying library likely has a bug.
+ return EIO;
+ }
+ return 0;
+ case EINTR:
+ case ERANGE:
+ // Retry on these errors.
+ // EINTR: a signal was handled and this thread was allowed to continue.
+ // ERANGE: the buffer was not big enough.
+ break;
+ default:
+ // Lookup failed.
+ return getgrgid_error_translate(ret);
}
- ginfo->buf = nbuf;
- ginfo->buf_sz = buf_sz;
}
+ // Did not succeed after the retries. Return the last error.
+ return getgrgid_error_translate(ret);
}
#ifdef GROUP_TESTING
Modified:
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c
URL:
http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c?rev=1588949&r1=1588948&r2=1588949&view=diff
==============================================================================
---
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c
(original)
+++
hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c
Mon Apr 21 18:27:16 2014
@@ -28,6 +28,7 @@
#include <unistd.h>
#define INITIAL_GIDS_SIZE 32
+#define MAX_USER_LOOKUP_TRIES 5
struct hadoop_user_info *hadoop_user_info_alloc(void)
{
@@ -95,31 +96,48 @@ int hadoop_user_info_fetch(struct hadoop
const char *username)
{
struct passwd *pwd;
- int err;
+ int ret, i;
size_t buf_sz;
char *nbuf;
hadoop_user_info_clear(uinfo);
- for (;;) {
- do {
- pwd = NULL;
- err = getpwnam_r(username, &uinfo->pwd, uinfo->buf,
- uinfo->buf_sz, &pwd);
- } while ((!pwd) && (errno == EINTR));
- if (pwd) {
- return 0;
- }
- if (err != ERANGE) {
- return getpwnam_error_translate(errno);
+ for (i = 0, ret = 0; i < MAX_USER_LOOKUP_TRIES; i++) {
+ // If the previous call returned ERANGE, increase the buffer size
+ if (ret == ERANGE) {
+ buf_sz = uinfo->buf_sz * 2;
+ nbuf = realloc(uinfo->buf, buf_sz);
+ if (!nbuf) {
+ return ENOMEM;
+ }
+ uinfo->buf = nbuf;
+ uinfo->buf_sz = buf_sz;
}
- buf_sz = uinfo->buf_sz * 2;
- nbuf = realloc(uinfo->buf, buf_sz);
- if (!nbuf) {
- return ENOMEM;
+
+ // The following call returns errno. Reading the global errno wihtout
+ // locking is not thread-safe.
+ pwd = NULL;
+ ret = getpwnam_r(username, &uinfo->pwd, uinfo->buf,
+ uinfo->buf_sz, &pwd);
+ switch(ret) {
+ case 0:
+ if (!pwd) {
+ // The underlying library likely has a bug.
+ return EIO;
+ }
+ return 0;
+ case EINTR:
+ case ERANGE:
+ // Retry on these errors.
+ // EINTR: a signal was handled and this thread was allowed to continue.
+ // ERANGE: the buffer was not big enough.
+ break;
+ default:
+ // Lookup failed.
+ return getpwnam_error_translate(ret);
}
- uinfo->buf = nbuf;
- uinfo->buf_sz = buf_sz;
}
+ // Did not succeed after the retries. Return the last error.
+ return getpwnam_error_translate(ret);
}
static int put_primary_gid_first(struct hadoop_user_info *uinfo)
