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