RFR (S): 8129786: Buffer overrun when passing long not existing option in JDK 9

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Wed Jul 1 16:38:53 UTC 2015


Hello Dan,

Thank you for review! Updated webrev: 
http://cr.openjdk.java.net/~ddmitriev/8129786/webrev.01/ 
<http://cr.openjdk.java.net/%7Eddmitriev/8129786/webrev.01/>
Please see my comments inline.

Regards,
Dmitry

On 01.07.2015 16:49, Daniel D. Daugherty wrote:
> > Webrev: http://cr.openjdk.java.net/~ddmitriev/8129786/webrev.00/
>
> src/share/vm/runtime/arguments.cpp
>     L840:   if (arg_len <= BUFLEN) {
>         Please add a comment. Perhaps:
>
>         // Only make the obsolete check for valid arguments.
Done!
>
>     L843:     strncpy(stripped_argname, argname, arg_len);
>     L844:     stripped_argname[arg_len] = '\0'; //strncpy doesn't null 
> terminate.
>         This is not due to your change but this comment isn't quite 
> right.
>         Perhaps:
>
>             stripped_argname[arg_len] = '\0';  // strncpy may not null 
> terminate.
>
>         strncpy() null terminates if the length of 'argname' is
>         less than arg_len. Also added one more space after ';'
>         and added a space after '//'.
Done!
>
>     L847:       char version[256];
>         Can you change this '256' to BUFLEN+1 also?
I prefer to leave it as is, because there are no relationship between 
'version' and BUFLEN. BUFLEN is used for parsing options, thus I use it 
for stripped_argname. But I think it will be better not to use it for 
'version'.

>
> test/runtime/CommandLine/TestLongUnrecognizedVMOption.java
>     L27:  * @summary Verify that JVM correctly process very long 
> unregnized VM option
>         Typo: 'process' -> 'processes'
>         Typo: 'unregnized' -> 'unrecognized'
>
Done!
> L29:  * @modules java.management
>         Why this module?
I got this dependency from script which used to find dependencies of 
test code.
>
>     L49: extra blank line at the end; jcheck may not like this
>
Done!
> Thumbs up. I don't need to see another webrev if you
> decide to fix these minor nits.
>
> Dan
>
>
> On 7/1/15 6:49 AM, Dmitry Dmitriev wrote:
>> Hello,
>>
>> Please review this small fix and new test. Also, I need a sponsor for 
>> this fix, who can push it.
>>
>> In this fix logic for stripped_argname was put into "if (arg_len <= 
>> BUFLEN)" statement. stripped_argname is used only to check is option 
>> is newly obsolete. Since valid VM option should be not bigger than 
>> 255 characters(BUFLEN value), then obsolete_jvm_flags contains only 
>> options with strlen <= BUFLEN.
>>
>> Webrev: http://cr.openjdk.java.net/~ddmitriev/8129786/webrev.00/ 
>> <http://cr.openjdk.java.net/%7Eddmitriev/8129786/webrev.00/>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8129786
>> Tested: JPRT(with new test), hotspot all & vm.quick
>>
>> Thanks,
>> Dmitry



More information about the hotspot-runtime-dev mailing list