HADOOP-9439. JniBasedUnixGroupsMapping: fix some crash bugs (Colin Patrick McCabe)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2.1-beta@1496117 13f79535-47bb-0310-9956-ffa450edef68

Author: cmccabe

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -256,6 +256,9 @@ Release 2.1.0-beta - UNRELEASED
     HADOOP-9624. TestFSMainOperationsLocalFileSystem failed when the Hadoop test
     root path has "X" in its name. (Xi Fang via cnauroth)
 
+    HADOOP-9439.  JniBasedUnixGroupsMapping: fix some crash bugs. (Colin
+    Patrick McCabe)
+
   BREAKDOWN OF HADOOP-8562 SUBTASKS AND RELATED JIRAS
 
     HADOOP-8924. Hadoop Common creating package-info.java must not depend on

hadoop-common-project/hadoop-common/src/CMakeLists.txt

@@ -180,7 +180,8 @@ add_dual_library(hadoop
     ${D}/net/unix/DomainSocket.c
     ${D}/security/JniBasedUnixGroupsMapping.c
     ${D}/security/JniBasedUnixGroupsNetgroupMapping.c
-    ${D}/security/getGroup.c
+    ${D}/security/hadoop_group_info.c
+    ${D}/security/hadoop_user_info.c
     ${D}/util/NativeCodeLoader.c
     ${D}/util/NativeCrc32.c
     ${D}/util/bulk_crc32.c

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/nativeio/NativeIO.java

@@ -98,7 +98,7 @@ public static class POSIX {
 
     static final String WORKAROUND_NON_THREADSAFE_CALLS_KEY =
       "hadoop.workaround.non.threadsafe.getpwuid";
-    static final boolean WORKAROUND_NON_THREADSAFE_CALLS_DEFAULT = false;
+    static final boolean WORKAROUND_NON_THREADSAFE_CALLS_DEFAULT = true;
 
     private static long cacheTimeout = -1;
 

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/JniBasedUnixGroupsMapping.java

@@ -40,22 +40,44 @@ public class JniBasedUnixGroupsMapping implements GroupMappingServiceProvider {
 
   private static final Log LOG = 
     LogFactory.getLog(JniBasedUnixGroupsMapping.class);
-  
-  native String[] getGroupForUser(String user);
-  
+
   static {
     if (!NativeCodeLoader.isNativeCodeLoaded()) {
       throw new RuntimeException("Bailing out since native library couldn't " +
         "be loaded");
     }
+    anchorNative();
     LOG.debug("Using JniBasedUnixGroupsMapping for Group resolution");
   }
 
+  /**
+   * Set up our JNI resources.
+   *
+   * @throws                 RuntimeException if setup fails.
+   */
+  native static void anchorNative();
+
+  /**
+   * Get the set of groups associated with a user.
+   *
+   * @param username           The user name
+   *
+   * @return                   The set of groups associated with a user.
+   */
+  native static String[] getGroupsForUser(String username);
+
+  /**
+   * Log an error message about a group.  Used from JNI.
+   */
+  static private void logError(int groupId, String error) {
+    LOG.error("error looking up the name of group " + groupId + ": " + error);
+  }
+
   @Override
   public List<String> getGroups(String user) throws IOException {
     String[] groups = new String[0];
     try {
-      groups = getGroupForUser(user);
+      groups = getGroupsForUser(user);
     } catch (Exception e) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Error getting groups for " + user, e);

hadoop-common-project/hadoop-common/src/main/native/src/exception.c

@@ -107,3 +107,12 @@ jthrowable newIOException(JNIEnv* env, const char *fmt, ...)
   va_end(ap);
   return jthr;
 }
+
+const char* terror(int errnum)
+{
+  if ((errnum < 0) || (errnum >= sys_nerr)) {
+    return "unknown error.";
+  }
+  return sys_errlist[errnum];
+}
+

hadoop-common-project/hadoop-common/src/main/native/src/exception.h

@@ -79,4 +79,12 @@ jthrowable newRuntimeException(JNIEnv* env, const char *fmt, ...)
 jthrowable newIOException(JNIEnv* env, const char *fmt, ...)
     __attribute__((format(printf, 2, 3)));
 
+/**
+ * Thread-safe strerror alternative.
+ *
+ * @param errnum        Error number.
+ * @return              Statically allocated error string.
+ */
+const char* terror(int errnum);
+
 #endif

hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/io/nativeio/NativeIO.c

@@ -59,7 +59,7 @@ static jmethodID nioe_ctor;
 // the monitor used for working around non-threadsafe implementations
 // of getpwuid_r, observed on platforms including RHEL 6.0.
 // Please see HADOOP-7156 for details.
-static jobject pw_lock_object;
+jobject pw_lock_object;
 
 // Internal functions
 static void throw_ioe(JNIEnv* env, int errnum);

hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c

@@ -107,14 +107,6 @@ static jthrowable newSocketException(JNIEnv *env, int errnum,
   return jthr;
 }
 
-static const char* terror(int errnum)
-{
-  if ((errnum < 0) || (errnum >= sys_nerr)) {
-    return "unknown error.";
-  }
-  return sys_errlist[errnum];
-}
-
 /**
  * Flexible buffer that will try to fit data on the stack, and fall back
  * to the heap if necessary.

hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMapping.c

@@ -30,88 +30,173 @@
 #include <pwd.h>
 #include <string.h>
 
+#include "exception.h"
 #include "org_apache_hadoop_security_JniBasedUnixGroupsMapping.h"
 #include "org_apache_hadoop.h"
+#include "hadoop_group_info.h"
+#include "hadoop_user_info.h"
 
-static jobjectArray emptyGroups = NULL;
+static jmethodID g_log_error_method;
 
-JNIEXPORT jobjectArray JNICALL 
-Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_getGroupForUser 
-(JNIEnv *env, jobject jobj, jstring juser) {
-  extern int getGroupIDList(const char *user, int *ngroups, gid_t **groups);
-  extern int getGroupDetails(gid_t group, char **grpBuf);
-  const char *cuser = NULL;
-  jobjectArray jgroups = NULL;
-  int error = -1;
+static jclass g_string_clazz;
 
-  if (emptyGroups == NULL) {
-    jobjectArray lEmptyGroups = (jobjectArray)(*env)->NewObjectArray(env, 0,
-            (*env)->FindClass(env, "java/lang/String"), NULL);
-    if (lEmptyGroups == NULL) {
-      goto cleanup;
-    }
-    emptyGroups = (*env)->NewGlobalRef(env, lEmptyGroups);
-    if (emptyGroups == NULL) {
-      goto cleanup;
-    }
+extern jobject pw_lock_object;
+
+JNIEXPORT void JNICALL
+Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_anchorNative(
+JNIEnv *env, jclass clazz)
+{
+  jobject string_clazz;
+
+  g_log_error_method = (*env)->GetStaticMethodID(env, clazz, "logError",
+        "(ILjava/lang/String;)V");
+  if (!g_log_error_method) {
+    return; // an exception has been raised
   }
-  char *grpBuf = NULL;
-  cuser = (*env)->GetStringUTFChars(env, juser, NULL);
-  if (cuser == NULL) {
-    goto cleanup;
+  string_clazz = (*env)->FindClass(env, "java/lang/String");
+  if (!string_clazz) {
+    return; // an exception has been raised
   }
-
-  /*Get the number of the groups, and their IDs, this user belongs to*/
-  gid_t *groups = NULL;
-  int ngroups = 0;
-  error = getGroupIDList(cuser, &ngroups, &groups);
-  if (error != 0) {
-    goto cleanup; 
+  g_string_clazz = (*env)->NewGlobalRef(env, string_clazz);
+  if (!g_string_clazz) {
+    jthrowable jthr = newRuntimeException(env,
+        "JniBasedUnixGroupsMapping#anchorNative: failed to make "
+        "a global reference to the java.lang.String class\n");
+    (*env)->Throw(env, jthr);
+    return;
   }
+}
 
-  jgroups = (jobjectArray)(*env)->NewObjectArray(env, ngroups, 
-            (*env)->FindClass(env, "java/lang/String"), NULL);
-  if (jgroups == NULL) {
-    error = -1;
-    goto cleanup; 
+/**
+ * Log an error about a failure to look up a group ID
+ *
+ * @param env   The JNI environment
+ * @param clazz JniBasedUnixGroupsMapping class
+ * @param gid   The gid we failed to look up
+ * @param ret   Failure code
+ */
+static void logError(JNIEnv *env, jclass clazz, jint gid, int ret)
+{
+  jstring error_msg;
+
+  error_msg = (*env)->NewStringUTF(env, terror(ret));
+  if (!error_msg) {
+    (*env)->ExceptionClear(env);
+    return;
+  }
+  (*env)->CallStaticVoidMethod(env, clazz, g_log_error_method, gid, error_msg);
+  if ((*env)->ExceptionCheck(env)) {
+    (*env)->ExceptionClear(env);
+    return;
   }
+  (*env)->DeleteLocalRef(env, error_msg);
+}
 
-  /*Iterate over the groupIDs and get the group structure for each*/
-  int i = 0;
-  for (i = 0; i < ngroups; i++) {
-    error = getGroupDetails(groups[i],&grpBuf);
-    if (error != 0) {
-      goto cleanup;
-    }
-    jstring jgrp = (*env)->NewStringUTF(env, ((struct group*)grpBuf)->gr_name);
-    if (jgrp == NULL) {
-      error = -1;
-      goto cleanup;
+JNIEXPORT jobjectArray JNICALL 
+Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_getGroupsForUser 
+(JNIEnv *env, jclass clazz, jstring jusername)
+{
+  const char *username = NULL;
+  struct hadoop_user_info *uinfo = NULL;
+  struct hadoop_group_info *ginfo = NULL;
+  jstring jgroupname = NULL;
+  int i, ret, nvalid;
+  int pw_lock_locked = 0;
+  jobjectArray jgroups = NULL, jnewgroups = NULL;
+
+  if (pw_lock_object != NULL) {
+    if ((*env)->MonitorEnter(env, pw_lock_object) != JNI_OK) {
+      goto done; // exception thrown
     }
-    (*env)->SetObjectArrayElement(env, jgroups,i,jgrp);
-    free(grpBuf);
-    grpBuf = NULL;
+    pw_lock_locked = 1;
   }
-
-cleanup:
-  if (error == ENOMEM) {
+  username = (*env)->GetStringUTFChars(env, jusername, NULL);
+  if (username == NULL) {
+    goto done; // exception thrown
+  }
+  uinfo = hadoop_user_info_alloc();
+  if (!uinfo) {
     THROW(env, "java/lang/OutOfMemoryError", NULL);
+    goto done;
+  }
+  ret = hadoop_user_info_fetch(uinfo, username);
+  if (ret == ENOENT) {
+    jgroups = (*env)->NewObjectArray(env, 0, g_string_clazz, NULL);
+    goto done;
+  }
+  ginfo = hadoop_group_info_alloc();
+  if (!ginfo) {
+    THROW(env, "java/lang/OutOfMemoryError", NULL);
+    goto done;
+  }
+  ret = hadoop_user_info_getgroups(uinfo);
+  if (ret) {
+    if (ret == ENOMEM) {
+      THROW(env, "java/lang/OutOfMemoryError", NULL);
+    } else {
+      char buf[128];
+      snprintf(buf, sizeof(buf), "getgrouplist error %d (%s)",
+               ret, terror(ret));
+      THROW(env, "java/lang/RuntimeException", buf);
+    }
+    goto done;
+  }
+  jgroups = (jobjectArray)(*env)->NewObjectArray(env, uinfo->num_gids,
+                                                 g_string_clazz, NULL);
+  for (nvalid = 0, i = 0; i < uinfo->num_gids; i++) {
+    ret = hadoop_group_info_fetch(ginfo, uinfo->gids[i]);
+    if (ret) {
+      logError(env, clazz, uinfo->gids[i], ret);
+    } else {
+      jgroupname = (*env)->NewStringUTF(env, ginfo->group.gr_name);
+      if (!jgroupname) { // exception raised
+        (*env)->DeleteLocalRef(env, jgroups);
+        jgroups = NULL;
+        goto done;
+      }
+      (*env)->SetObjectArrayElement(env, jgroups, nvalid++, jgroupname);
+      // We delete the local reference once the element is in the array.
+      // This is OK because the array has a reference to it.
+      // Technically JNI only mandates that the JVM allow up to 16 local
+      // references at a time  (though many JVMs allow more than that.)
+      (*env)->DeleteLocalRef(env, jgroupname);
+    }
+  }
+  if (nvalid != uinfo->num_gids) {
+    // If some group names could not be looked up, allocate a smaller array
+    // with just the entries that could be resolved.  Java has no equivalent to
+    // realloc, so we have to do this manually.
+    jnewgroups = (jobjectArray)(*env)->NewObjectArray(env, nvalid,
+            (*env)->FindClass(env, "java/lang/String"), NULL);
+    if (!jnewgroups) { // exception raised
+      (*env)->DeleteLocalRef(env, jgroups);
+      jgroups = NULL;
+      goto done;
+    }
+    for (i = 0; i < nvalid; i++) {
+      jgroupname = (*env)->GetObjectArrayElement(env, jgroups, i);
+      (*env)->SetObjectArrayElement(env, jnewgroups, i, jgroupname);
+      (*env)->DeleteLocalRef(env, jgroupname);
+    }
+    (*env)->DeleteLocalRef(env, jgroups);
+    jgroups = jnewgroups;
   }
-  if (error == ENOENT) {
-    THROW(env, "java/io/IOException", "No entry for user");
+
+done:
+  if (pw_lock_locked) {
+    (*env)->MonitorExit(env, pw_lock_object);
   }
-  if (groups != NULL) {
-    free(groups);
+  if (username) {
+    (*env)->ReleaseStringUTFChars(env, jusername, username);
   }
-  if (grpBuf != NULL) {
-    free(grpBuf);
+  if (uinfo) {
+    hadoop_user_info_free(uinfo);
   }
-  if (cuser != NULL) {
-    (*env)->ReleaseStringUTFChars(env, juser, cuser);
+  if (ginfo) {
+    hadoop_group_info_free(ginfo);
   }
-  if (error == 0) {
-    return jgroups;
-  } else {
-    return emptyGroups;
+  if (jgroupname) {
+    (*env)->DeleteLocalRef(env, jgroupname);
   }
+  return jgroups;
 }

hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/security/JniBasedUnixGroupsMappingWin.c

@@ -25,6 +25,13 @@
 
 static jobjectArray emptyGroups = NULL;
 
+JNIEXPORT void JNICALL
+Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_anchorNative(
+JNIEnv *env, jclass clazz)
+{
+  // no-op until full port of HADOOP-9439 to Windows is available
+}
+
 /*
  * Throw a java.IO.IOException, generating the message from errno.
  */
@@ -57,8 +64,8 @@ static void throw_ioexception(JNIEnv* env, DWORD errnum)
 }
 
 JNIEXPORT jobjectArray JNICALL 
-Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_getGroupForUser 
-(JNIEnv *env, jobject jobj, jstring juser) {
+Java_org_apache_hadoop_security_JniBasedUnixGroupsMapping_getGroupsForUser
+(JNIEnv *env, jclass clazz, jstring juser) {
   const WCHAR *user = NULL;
   jobjectArray jgroups = NULL;
   DWORD dwRtnCode = ERROR_SUCCESS;