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

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
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

Reply | Threaded
Open this post in threaded view
|

Re: 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, [hidden email] 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
>
Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
Thanks for the quick review, David!
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, [hidden email] 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
>>

Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
In reply to this post by David Holmes
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, [hidden email] 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
>>

Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer
I actually took a look at it the other day but never responded. I was
wondering if we really want to print a message here. I didn't see any
other cases of doing this. Also, if we are out of native memory, do we
really want to continue?

Chris

On 10/18/17 12:15 PM, [hidden email] 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, [hidden email] 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
>>>
>


Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@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, [hidden email] 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, [hidden email] 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
>>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
On 10/18/17 13:02, [hidden email] 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, [hidden email] 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, [hidden email] 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
>>>>>
>>>
>>
>>
>

Reply | Threaded
Open this post in threaded view
|

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

Chris Plummer
On 10/18/17 1:10 PM, [hidden email] wrote:

> On 10/18/17 13:02, [hidden email] 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. :)
Ok. Changes are fine.

thanks,

Chris

>
> Thanks,
> Serguei
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> Chris
>>>
>>> On 10/18/17 12:15 PM, [hidden email] 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, [hidden email] 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
>>>>>>
>>>>
>>>
>>>
>>
>


Reply | Threaded
Open this post in threaded view
|

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

serguei.spitsyn@oracle.com
On 10/18/17 13:33, Chris Plummer wrote:

> On 10/18/17 1:10 PM, [hidden email] wrote:
>> On 10/18/17 13:02, [hidden email] 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. :)
> Ok. Changes are fine.

Thanks a lot, Chris!
Serguei

>
> thanks,
>
> Chris
>>
>> Thanks,
>> Serguei
>>
>>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>>
>>>> Chris
>>>>
>>>> On 10/18/17 12:15 PM, [hidden email] 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, [hidden email] 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
>>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>
>