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

[David Holmes]

Hi,

Was there a reason you had to move the existing arrays before extending 
the amd one with the missing values?

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
>