RFR (XS): 8175510 Null pointer dereference in getModuleObject of JPLISAgent.c:790

[David Holmes]

Hi Serguei,

Seems quite reasonable.

Reviewed.

Thanks,
David

On 13/10/2017 8:21 AM, serguei.spitsyn at oracle.com wrote:
> Please, review a fix for the Parfait bug:
> https://bugs.openjdk.java.net/browse/JDK-8175510
> 
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2017/hotspot/8175510-jplis-parfait.1/
> 
> 
> Summary:
> 
>    This is the main fragment from the Parfait report:
> 
> 
>         getModuleObject
> 
> FileExpandCollapseLine 
> #jdk/src/java.instrument/share/native/libinstrument/JPLISAgent.c
> jdk-9+180-JDK9_linux
> 
> 783. 	
> 
>      int len = (last_slash == NULL) ? 0 : (int)(last_slash - cname);
> 
> 784. 	
> 
>      char* pkg_name_buf = (char*)malloc(len + 1);
> 
> 785. 	
> 786. 	
> 
>      jplis_assert_msg(pkg_name_buf != NULL, "OOM error in native tmp buffer allocation");
> 
> Pointer checked against constant 'NULL' but does not protect the 
> dereference.
> 787. 	
> 
>      if (last_slash != NULL) {
> 
> 788. 	
> 
>          strncpy(pkg_name_buf, cname, len);
> 
> 789. 	
> 
>      }
> 
> 790. 	
> 
>      pkg_name_buf[len] = '\0';
> 
> *Null pointer dereference not protected by null check*
> Write to pointer pkg_name_buf that could be constant 'NULL'
> 
> 
>    The malloc can return NULL in a case of OOME.
>    The assert at L786 checks the returned pointer for NULL but does not 
> protect the dereference at L790.
>    The fix is to replace the assert with printing a error message and 
> returning with NULL from the getModuleObject().
>    It must be safe as the returned result is passed to the 
> sun.instrument.InstrumentationImpl.transform()
>    which handles null passed as in the module parameter.
> 
> Thanks,
> Serguei
>