RFR: Out-of-bounds access in cpu_family_description()

[Jinke Fan]

On 2019/4/12 14:20, David Holmes wrote:
> Hi,
> 
> Was there a reason you had to move the existing arrays before extending 
> the amd one with the missing values?
Yes, because the addition uses sizeof to control the boundary access.
If the arrays's defined were behind the sizeof,it causes a compilation 
error: "error: invalid application of 'sizeof’ to incomplete type"

Best Regards!
Fanjinke
> 
> Thanks,
> David
> 
> On 12/04/2019 3:47 pm, Jinke Fan wrote:
>> Hi David,
>>      In VM_Version_Ext::cpu_family_description has out-of-bounds
>> access on AMD 17h (EPYC) processor.
>>
>> const char* VM_Version_Ext::cpu_family_description(void) {
>>
>> On AMD 17h (EPYC) processor extended_cpu_family() will return 23,
>> but array _family_id_amd only has 17 members.
>>
>>    int cpu_family_id = extended_cpu_family();
>>    if (is_amd()) {
>>      return _family_id_amd[cpu_family_id];
>>    }
>> ...
>> }
>>
>> Result of testcase TestCPUInformation.java on AMD Zen:
>> ----------System.out:(15/1615)----------
>> ...
>> Family: 386 (0x17), Model: <unknown> (0x1), Stepping: 0x1
>> Ext. family: 0x8, Ext. model: 0x0, Type: 0x0, Signature: 0x00800f11
>> ...
>> }
>>
>> The “386” string is incorrectly and comes from Illegal access.
>>
>> The patch is based on the original repository:
>> hg.openjdk.java.net/jdk/jdk
>>
>> changeset:   54520:f48312257bc6
>> tag:         tip
>> user:        vromero
>> date:        Thu Apr 11 22:56:11 2019 -0400
>> summary:     8222151: refactoring: enhancements to 
>> java.lang.Class::methodToString and java.lang.Class::getTypeName
>>
>> *Patch
>> The out of hg diff -g:
>> diff --git a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp 
>> b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>> --- a/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>> +++ b/src/hotspot/cpu/x86/vm_version_ext_x86.cpp
>> @@ -262,6 +262,52 @@
>>   int VM_Version_Ext::_no_of_cores = 0;
>>   int VM_Version_Ext::_no_of_packages = 0;
>>
>> +const char* const VM_Version_Ext::_family_id_intel[] = {
>> +  "8086/8088",
>> +  "",
>> +  "286",
>> +  "386",
>> +  "486",
>> +  "Pentium",
>> +  "Pentium Pro",   //or Pentium-M/Woodcrest depeding on model
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "Pentium 4"
>> +};
>> +
>> +const char* const VM_Version_Ext::_family_id_amd[] = {
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "5x86",
>> +  "K5/K6",
>> +  "Athlon/AthlonXP",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "Opteron/Athlon64",
>> +  "Opteron QC/Phenom",  // Barcelona et.al.
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "",
>> +  "Zen"
>> +};
>> +
>>   void VM_Version_Ext::initialize(void) {
>>     ResourceMark rm;
>>
>> @@ -401,15 +447,19 @@
>>   }
>>
>>   const char* VM_Version_Ext::cpu_family_description(void) {
>> -  int cpu_family_id = extended_cpu_family();
>> +  uint32_t cpu_family_id = extended_cpu_family();
>>     if (is_amd()) {
>> -    return _family_id_amd[cpu_family_id];
>> +    if (cpu_family_id < 
>> sizeof(_family_id_amd)/sizeof(_family_id_amd[0])) {
>> +      return _family_id_amd[cpu_family_id];
>> +    }
>>     }
>>     if (is_intel()) {
>>       if (cpu_family_id == CPU_FAMILY_PENTIUMPRO) {
>>         return cpu_model_description();
>>       }
>> -    return _family_id_intel[cpu_family_id];
>> +    if (cpu_family_id < 
>> sizeof(_family_id_intel)/sizeof(_family_id_intel[0])) {
>> +      return _family_id_intel[cpu_family_id];
>> +    }
>>     }
>>     if (is_hygon()) {
>>       return "Dhyana";
>> @@ -705,44 +755,6 @@
>>     return _max_qualified_cpu_frequency;
>>   }
>>
>> -const char* const VM_Version_Ext::_family_id_intel[] = {
>> -  "8086/8088",
>> -  "",
>> -  "286",
>> -  "386",
>> -  "486",
>> -  "Pentium",
>> -  "Pentium Pro",   //or Pentium-M/Woodcrest depeding on model
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "Pentium 4"
>> -};
>> -
>> -const char* const VM_Version_Ext::_family_id_amd[] = {
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "5x86",
>> -  "K5/K6",
>> -  "Athlon/AthlonXP",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "",
>> -  "Opteron/Athlon64",
>> -  "Opteron QC/Phenom"  // Barcelona et.al.
>> -};
>>   // Partially from Intel 64 and IA-32 Architecture Software 
>> Developer's Manual,
>>   // September 2013, Vol 3C Table 35-1
>>   const char* const VM_Version_Ext::_model_id_pentium_pro[] = {
>>
>> *Test:
>> After patched,result of testcase TestCPUInformation.java on AMD Zen:
>> ----------System.out:(15/1615)----------
>> Event: jdk.CPUInformation {
>>    ...
>> Family: Zen (0x17), Model: <unknown> (0x1), Stepping: 0x1
>> Ext. family: 0x8, Ext. model: 0x0, Type: 0x0, Signature: 0x00800f11
>> Features: ebx: 0x4f400800, ecx: 0x7ed8320b, edx: 0x178bfbff
>>    ...
>> }
>>
>> Is there anything incorrectly?
>> Please let me know your comments.
>>
>> Best Regards！
>> Fanjinke
>>
>