RFR(XXS): 8144197: Possible use after free in Arguments::add_property function

[Dmitry Dmitriev]

Hi Goetz,

Thank you for the review!

No, I didn't get an oom there. I just look at some generated OOM log's 
and my eyes found "java_command" entry. I realized that java_command is 
used in OOM reporting and remember that I free it before allocating a 
new one in add_property function.

Dmitry

On 10.12.2015 12:56, Lindenmaier, Goetz wrote:
> Hi Dmitry,
>
> this looks good.
>
> How did you spot this, did you really get an oom there?
>
> Best regards,
>    Goetz.
>
>> -----Original Message-----
>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>> bounces at openjdk.java.net] On Behalf Of Dmitry Dmitriev
>> Sent: Thursday, December 10, 2015 10:13 AM
>> To: hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(XXS): 8144197: Possible use after free in
>> Arguments::add_property function
>>
>> Hello,
>>
>> Please, can I one more "r" review for that fix? Thank you!
>>
>> Dmitry
>>
>> On 01.12.2015 14:38, Dmitry Dmitriev wrote:
>>> Hello,
>>>
>>> Please, review this small fix. JDK-8132725 removes memory leak in
>>> Arguments::add_property function in
>>> hotspot/src/share/vm/runtime/arguments.cpp module, but introduce
>>> theoretical possibility of use after free of _java_command, because
>>> _java_command used in reporting OOM errors. This can happen if
>>> sun.java.command property is defined more than once and JVM can not
>>> allocate memory for new java command when processing second(or later)
>>> "sun.java.command" property. Thus sun.java.command can point to the
>>> freed memory when reporting OOM error, because memory for
>>> sun.java.command is freed before allocating new sun.java.command. Fix
>>> is simple - free old sun.java.command only after allocating memory for
>>> the new one. The similar thing is done for java.vendor.url.bug
>>> property in case if fatal error occurred during allocation of new
>>> java.vendor.url.bug property.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8144197
>>> webrev.00: http://cr.openjdk.java.net/~ddmitriev/8144197/webrev.00/
>>> <http://cr.openjdk.java.net/%7Eddmitriev/8144197/webrev.00/>
>>> Testing: locally, JPRT
>>>
>>> Thanks,
>>> Dmitry