RFR: 8229378: jdwp library loader in linker_md.c quietly truncates on buffer overflow

Thu Aug 15 11:38:34 UTC 2019

Hi Serguei, Daniel,

Good to hear you like the fix.

My intention with the testing was to make sure my change didn't break 
anything else. I didn't do a code paths check before I ran it though; 
saturation run.

As for writing a new test, I'm finding it tricky.

Here's the current flow:

Step 1) VM initialises.
Step 2) VM loads a couple of libraries and shuts down if one or more paths 
is too long in sun.boot.library.path.
Step 3) JDWP initializes
Step 4) JDWP loads a library and shuts down if one or more paths is too 
long in sun.boot.library.path.

As you can see, Step 2 prevents us from reaching Step 4 with a 
too-long-path (required to cause failure).

I worked around that with my webrev by disabling the bit in os.cpp that 
enacts Step 2.

Since my hack will be removed in the final webrev, we need another way to 
reach step 4.

So what we need to test this change, I believe, is a way to insert Step 
2.5) Change the property to include a too-long path.

This allows the VM to start up properly, but gives us the excessive path 
we need to test the jdwp fix.

Right now, I'm not seeing a way to do this outside of using the JNI.

1) shell script launches cpp file.
2) cpp starts vm without jdwp.
3) change the property.
4) call jdwp library-loading method directly.
5) check the return code.

This seems messy, but I'm not seeing a way to initialise jdwp from inside 
java code (which sounds better to me).

I welcome anyone who can think of a better way to do this.

Best Regards

Adam Farley 
IBM Runtimes

"serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on 
15/08/2019 09:25:36:

> From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
> To: Adam Farley8 <adam.farley at uk.ibm.com>
> Cc: Chris Plummer <chris.plummer at oracle.com>, 
> daniel.daugherty at oracle.com, serviceability-dev at openjdk.java.net
> Date: 15/08/2019 09:25
> Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c 
> quietly truncates on buffer overflow
> 
> Hi Adam,
> 
> The fix itself looks Okay to me.
> I'm not sure there is any test case in these test suites which 
> provide a coverage for it.
> It looks like you need to develop a unit jtreg unit test for this.
> 
> Thanks,
> Serguei
> 
> 
> On 8/13/19 09:28, Adam Farley8 wrote:
> Hi Serguei, Daniel, 
> 
> My testing was limited to the bug specific test case I mentioned, 
> and the following jdwp tests: 
> 
> test/jdk/com/sun/jdi/Jdwp*
> test/hotspot/jtreg/serviceability/jdwp 
> 
> Best Regards
> 
> Adam Farley 
> IBM Runtimes
> 
> 
> "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on 
> 13/08/2019 17:04:43:
> 
> > From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> 
> > To: daniel.daugherty at oracle.com, Adam Farley8 
> > <adam.farley at uk.ibm.com>, Chris Plummer <chris.plummer at oracle.com> 
> > Cc: serviceability-dev at openjdk.java.net 
> > Date: 13/08/2019 17:08 
> > Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c 
> > quietly truncates on buffer overflow 
> > 
> > Hi Adam,
> > 
> > I'm looking at your fix.
> > Also interested about your testing.
> > 
> > Thanks,
> > Serguei
> > 
> > On 8/13/19 08:48, Daniel D. Daugherty wrote: 
> > I don't see any information about how this change was tested...
> > Is there something on another email thread?
> > 
> > Dan
> > 
> 
> > On 8/13/19 11:41 AM, Adam Farley8 wrote: 
> > Hi Chris, 
> > 
> > Thanks! 
> > 
> > I understand we need a second reviewer/sponsor to get this change 
> > in. Any volunteers?
> > 
> > Best Regards
> > 
> > Adam Farley 
> > IBM Runtimes
> > 
> > 
> > Chris Plummer <chris.plummer at oracle.com> wrote on 12/08/2019 21:35:06:
> > 
> > > From: Chris Plummer <chris.plummer at oracle.com> 
> > > To: Adam Farley8 <adam.farley at uk.ibm.com>, serviceability-
> > dev at openjdk.java.net 
> > > Date: 12/08/2019 21:35 
> > > Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c 
> > > quietly truncates on buffer overflow 
> > > 
> > > Hi Adam,
> > > 
> > > It looks good to me.
> > > 
> > > thanks,
> > > 
> > > Chris
> > > 
> > > On 8/12/19 7:34 AM, Adam Farley8 wrote: 
> > > Hi All, 
> > > 
> > > This is a known bug, mentioned in a code comment. 
> > > 
> > > Here is the fix for that bug. 
> > > 
> > > Reviewers and sponsors requested. 
> > > 
> > > Short version: if you set sun.boot.library.path to 
> > > something beyond a system's max path length, the 
> > > current code will return an empty string (rather than 
> > > printing a useful error message and shutting down). 
> > > 
> > > This is also a problem if you've specified multiple 
> > > paths with a separator, as this code seems to wrongly 
> > > assess whether the *total* length exceeds max path 
> > > length. So two 200 char paths on windows will cause 
> > > failure, as the total length is 400 (which is beyond 
> > > max length for windows). 
> > > 
> > > Note that the os.cpp bit of the webrev will not be included 
> > > in the final webrev, it just makes this change trivially 
> > > testable. 
> > > 
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8229378 
> > > Webrev: http://cr.openjdk.java.net/~afarley/8229378/webrev/ 
> > > 
> > > 
> > > Best Regards
> > > 
> > > Adam Farley 
> > > IBM Runtimes
> > > 
> > > Unless stated otherwise above:
> > > IBM United Kingdom Limited - Registered in England and Wales with 
> > > number 741598. 
> > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
PO6 3AU
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with 
> > number 741598. 
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with 
> number 741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
3AU
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 
741598. 
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190815/38ea36ba/attachment.html>