Author: kihwal Date: Mon Apr 21 18:28:53 2014 New Revision: 1588950 URL: http://svn.apache.org/r1588950 Log: svn merge -c 1588949 merging from trunk to branch-2 to fix:HADOOP-10522. JniBasedUnixGroupMapping mishandles errors. Contributed by Kihwal Lee.
Modified: hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c Modified: hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1588950&r1=1588949&r2=1588950&view=diff ============================================================================== --- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/CHANGES.txt Mon Apr 21 18:28:53 2014 @@ -100,6 +100,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/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c?rev=1588950&r1=1588949&r2=1588950&view=diff ============================================================================== --- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c (original) +++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c Mon Apr 21 18:28:53 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/branches/branch-2/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/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c?rev=1588950&r1=1588949&r2=1588950&view=diff ============================================================================== --- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c (original) +++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_group_info.c Mon Apr 21 18:28:53 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/branches/branch-2/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/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c?rev=1588950&r1=1588949&r2=1588950&view=diff ============================================================================== --- hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c (original) +++ hadoop/common/branches/branch-2/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/hadoop_user_info.c Mon Apr 21 18:28:53 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)