8006005: Fix constant pool index validation and alignment trap for method parameter reflection
authorcoleenp
Mon, 14 Jan 2013 11:01:39 -0500
changeset 15194 a35093d73168
parent 15192 f71a6fb9b532
child 15195 6313e53c38a1
8006005: Fix constant pool index validation and alignment trap for method parameter reflection Summary: This patch addresses an alignment trap due to the storage format of method parameters data in constMethod. It also adds code to validate constant pool indexes for method parameters data. Reviewed-by: jrose, dholmes Contributed-by: eric.mccorkle@oracle.com
hotspot/src/share/vm/classfile/classFileParser.cpp
hotspot/src/share/vm/oops/constMethod.hpp
hotspot/src/share/vm/prims/jvm.cpp
hotspot/src/share/vm/runtime/reflection.cpp
--- a/hotspot/src/share/vm/classfile/classFileParser.cpp	Fri Jan 11 09:53:24 2013 -0800
+++ b/hotspot/src/share/vm/classfile/classFileParser.cpp	Mon Jan 14 11:01:39 2013 -0500
@@ -59,6 +59,7 @@
 #include "services/classLoadingService.hpp"
 #include "services/threadService.hpp"
 #include "utilities/array.hpp"
+#include "utilities/globalDefinitions.hpp"
 
 // We generally try to create the oops directly when parsing, rather than
 // allocating temporary data structures and copying the bytes twice. A
@@ -2148,9 +2149,21 @@
                                      cp, CHECK_(nullHandle));
     } else if (method_attribute_name == vmSymbols::tag_method_parameters()) {
       method_parameters_length = cfs->get_u1_fast();
+      // Track the actual size (note: this is written for clarity; a
+      // decent compiler will CSE and constant-fold this into a single
+      // expression)
+      u2 actual_size = 1;
       method_parameters_data = cfs->get_u1_buffer();
+      actual_size += 2 * method_parameters_length;
       cfs->skip_u2_fast(method_parameters_length);
+      actual_size += 4 * method_parameters_length;
       cfs->skip_u4_fast(method_parameters_length);
+      // Enforce attribute length
+      if (method_attribute_length != actual_size) {
+        classfile_parse_error(
+          "Invalid MethodParameters method attribute length %u in class file %s",
+          method_attribute_length, CHECK_(nullHandle));
+      }
       // ignore this attribute if it cannot be reflected
       if (!SystemDictionary::Parameter_klass_loaded())
         method_parameters_length = 0;
@@ -2297,7 +2310,10 @@
       elem[i].name_cp_index =
         Bytes::get_Java_u2(method_parameters_data);
       method_parameters_data += 2;
-      elem[i].flags = Bytes::get_Java_u4(method_parameters_data);
+      u4 flags = Bytes::get_Java_u4(method_parameters_data);
+      // This caused an alignment fault on Sparc, if flags was a u4
+      elem[i].flags_lo = extract_low_short_from_int(flags);
+      elem[i].flags_hi = extract_high_short_from_int(flags);
       method_parameters_data += 4;
     }
   }
--- a/hotspot/src/share/vm/oops/constMethod.hpp	Fri Jan 11 09:53:24 2013 -0800
+++ b/hotspot/src/share/vm/oops/constMethod.hpp	Mon Jan 14 11:01:39 2013 -0500
@@ -122,7 +122,12 @@
 class MethodParametersElement VALUE_OBJ_CLASS_SPEC {
  public:
   u2 name_cp_index;
-  u4 flags;
+  // This has to happen, otherwise it will cause SIGBUS from a
+  // misaligned u4 on some architectures (ie SPARC)
+  // because MethodParametersElements are only aligned mod 2
+  // within the ConstMethod container  u2 flags_hi;
+  u2 flags_hi;
+  u2 flags_lo;
 };
 
 
--- a/hotspot/src/share/vm/prims/jvm.cpp	Fri Jan 11 09:53:24 2013 -0800
+++ b/hotspot/src/share/vm/prims/jvm.cpp	Mon Jan 14 11:01:39 2013 -0500
@@ -1589,6 +1589,12 @@
   return NULL;
 JVM_END
 
+static void bounds_check(constantPoolHandle cp, jint index, TRAPS) {
+  if (!cp->is_within_bounds(index)) {
+    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Constant pool index out of bounds");
+  }
+}
+
 JVM_ENTRY(jobjectArray, JVM_GetMethodParameters(JNIEnv *env, jobject method))
 {
   JVMWrapper("JVM_GetMethodParameters");
@@ -1598,15 +1604,31 @@
   Handle reflected_method (THREAD, JNIHandles::resolve_non_null(method));
   const int num_params = mh->method_parameters_length();
 
-  if(0 != num_params) {
+  if (0 != num_params) {
+    // make sure all the symbols are properly formatted
+    for (int i = 0; i < num_params; i++) {
+      MethodParametersElement* params = mh->method_parameters_start();
+      int index = params[i].name_cp_index;
+      bounds_check(mh->constants(), index, CHECK_NULL);
+
+      if (0 != index && !mh->constants()->tag_at(index).is_utf8()) {
+        THROW_MSG_0(vmSymbols::java_lang_IllegalArgumentException(),
+                    "Wrong type at constant pool index");
+      }
+
+    }
+
     objArrayOop result_oop = oopFactory::new_objArray(SystemDictionary::reflect_Parameter_klass(), num_params, CHECK_NULL);
     objArrayHandle result (THREAD, result_oop);
 
-    for(int i = 0; i < num_params; i++) {
+    for (int i = 0; i < num_params; i++) {
       MethodParametersElement* params = mh->method_parameters_start();
-      Symbol* const sym = mh->constants()->symbol_at(params[i].name_cp_index);
+      // For a 0 index, give a NULL symbol
+      Symbol* const sym = 0 != params[i].name_cp_index ?
+        mh->constants()->symbol_at(params[i].name_cp_index) : NULL;
+      int flags = build_int_from_shorts(params[i].flags_lo, params[i].flags_hi);
       oop param = Reflection::new_parameter(reflected_method, i, sym,
-                                            params[i].flags, CHECK_NULL);
+                                            flags, CHECK_NULL);
       result->obj_at_put(i, param);
     }
     return (jobjectArray)JNIHandles::make_local(env, result());
@@ -1830,13 +1852,6 @@
 JVM_END
 
 
-static void bounds_check(constantPoolHandle cp, jint index, TRAPS) {
-  if (!cp->is_within_bounds(index)) {
-    THROW_MSG(vmSymbols::java_lang_IllegalArgumentException(), "Constant pool index out of bounds");
-  }
-}
-
-
 JVM_ENTRY(jclass, JVM_ConstantPoolGetClassAt(JNIEnv *env, jobject obj, jobject unused, jint index))
 {
   JVMWrapper("JVM_ConstantPoolGetClassAt");
@@ -1851,7 +1866,6 @@
 }
 JVM_END
 
-
 JVM_ENTRY(jclass, JVM_ConstantPoolGetClassAtIfLoaded(JNIEnv *env, jobject obj, jobject unused, jint index))
 {
   JVMWrapper("JVM_ConstantPoolGetClassAtIfLoaded");
--- a/hotspot/src/share/vm/runtime/reflection.cpp	Fri Jan 11 09:53:24 2013 -0800
+++ b/hotspot/src/share/vm/runtime/reflection.cpp	Mon Jan 14 11:01:39 2013 -0500
@@ -862,7 +862,15 @@
 
 oop Reflection::new_parameter(Handle method, int index, Symbol* sym,
                               int flags, TRAPS) {
-  Handle name = java_lang_String::create_from_symbol(sym, CHECK_NULL);
+  Handle name;
+
+  // A null symbol here translates to the empty string
+  if(NULL != sym) {
+    name = java_lang_String::create_from_symbol(sym, CHECK_NULL);
+  } else {
+    name = java_lang_String::create_from_str("", CHECK_NULL);
+  }
+
   Handle rh = java_lang_reflect_Parameter::create(CHECK_NULL);
   java_lang_reflect_Parameter::set_name(rh(), name());
   java_lang_reflect_Parameter::set_modifiers(rh(), flags);