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

[serguei.spitsyn at oracle.com]

On 10/18/17 13:02, serguei.spitsyn at oracle.com wrote:
> 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.

Sorry for the typo, I had to say the JPLIS agent. :)

Thanks,
Serguei

>
>
> 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
>>>>>
>>>
>>
>>
>