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

[serguei.spitsyn at oracle.com]

Hi Chris,


On 10/18/17 12:34, Chris Plummer wrote:
> I actually took a look at it the other day but never responded.

Thank you for reviewing it!

> I was wondering if we really want to print a message here.I didn't see 
> any other cases of doing this.

I see no harm to print it in this particular case at least to have some 
clue about what is going on.


> Also, if we are out of native memory, do we really want to continue?

In this case, the VM itself will be aborted with a big probability.
I do not see cases where the JDWP agent is aborted other than at 
initialization.


Thanks,
Serguei


>
> Chris
>
> On 10/18/17 12:15 PM, serguei.spitsyn at oracle.com wrote:
>> Is anyone interested to review this simple fix?
>> Otherwise, I'd suggest to push it with one review from David under 
>> trivial fix rule.
>>
>> Thanks,
>> Serguei
>>
>>
>> On 10/12/17 18:01, David Holmes wrote:
>>> 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
>>>>
>>
>
>