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

[Daniel D. Daugherty]

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

     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 '//'.

     L847:       char version[256];
         Can you change this '256' to BUFLEN+1 also?

test/runtime/CommandLine/TestLongUnrecognizedVMOption.java
     L27:  * @summary Verify that JVM correctly process very long 
unregnized VM option
         Typo: 'process' -> 'processes'
         Typo: 'unregnized' -> 'unrecognized'

     L29:  * @modules java.management
         Why this module?

     L49: extra blank line at the end; jcheck may not like this

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