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

Adam Farley8 adam.farley at uk.ibm.com
Mon Sep 9 16:53:50 UTC 2019 

Hi Serguei,

Apologies for the delay.

The errors have all been fixed, and the requested tests mostly passed, 
windows and linux.

No test group had more failures post-fix than pre-fix, so I'm calling that 
a win.

The new webrev can be found here:

http://cr.openjdk.java.net/~afarley/8229378.2/webrev

Best Regards

Adam Farley 
IBM Runtimes


"serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on 
29/08/2019 19:38:02:

> 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: 29/08/2019 19:38
> Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c 
> quietly truncates on buffer overflow
> 
> Hi Adam,
> 
> Okay, thanks!
> Serguei
> 
> 
> On 8/29/19 06:26, Adam Farley8 wrote:
> Hi Serguei, 
> 
> I haven't actually run a fastdebug build before. Will do that now 
> and address the issues. 
> 
> Once done, I'll re-run the tests I ran, and also the tests you've 
> listed below. 
> 
> Can you advise on how "good coverage" is determined, so I know for 
> future bug fixes? 
> 
> As for the up-to-date-ness, I'll update the build before doing the 
above. 
> 
> Expect a webrev once all of this is complete.
> 
> Best Regards
> 
> Adam Farley 
> IBM Runtimes
> 
> 
> "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on 
> 29/08/2019 03:54:56:
> 
> > 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: 29/08/2019 04:23 
> > Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c 
> > quietly truncates on buffer overflow 
> > 
> > Hi Adam,
> > 
> > Sorry for the latency.
> > I was in process to build, test and push your fix and got the 
> > fastdebug build errors below.
> > 
> > So, my question is if you've ever built the fastdebug version.
> > This change is in the system-dependent code, so it has to be tested 
> > on both Unix and Windows.
> > 
> > > 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
> > 
> > This set of tests does not provide a good coverage.
> > To make sure nothing is broken you need to run the the 
test/jdk/com/sun/jdi
> > and also the following vmTestbase tests:
> > 
> >    test/hotspot/jtreg/vmTestbase/nsk/jdi
> >    test/hotspot/jtreg/vmTestbase/nsk/jdb
> >    test/hotspot/jtreg/vmTestbase/nsk/jdwp
> > 
> > BTW, your current webrev is not up-to-date:
> >   http://cr.openjdk.java.net/~afarley/8229378/webrev/
> > 
> > I guess, the change in the src/hotspot/share/runtime/os.cpp became
> > obsolete after your previous fix that was already pushed.
> > 
> > Thanks,
> > Serguei
> > 
> > . . .
> > In file included from /scratch/sspitsyn/jdk14.1/open/src/
> > jdk.jdwp.agent/unix/native/libjdwp/linker_md.c:37:0:
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c: In function ‘dll_build_name’:
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:23: error: ‘Do’ undeclared (first use in this 
function)
> >      #define strdup(p) Do not use this interface.
> >                        ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of macro ‘strdup’
> >      paths_copy = strdup(paths);
> >                   ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:23: note: each undeclared identifier is reported 
> > only once for each function it appears in
> >      #define strdup(p) Do not use this interface.
> >                        ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of macro ‘strdup’
> >      paths_copy = strdup(paths);
> >                   ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:46:26: error: expected ‘;’ before ‘not’
> >      #define strdup(p) Do not use this interface.
> >                           ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:51:18: note: in expansion of macro ‘strdup’
> >      paths_copy = strdup(paths);
> >                   ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/share/native/
> > libjdwp/util.h:38:24: error: expected ‘;’ before ‘not’
> >      #define free(p) Do not use this interface.
> >                         ^
> > /scratch/sspitsyn/jdk14.1/open/src/jdk.jdwp.agent/unix/native/
> > libjdwp/linker_md.c:71:5: note: in expansion of macro ‘free’
> >      free(paths_copy);
> >      ^
> > gmake[3]: *** [/scratch/sspitsyn/jdk14.1/build/linux-x86_64-server-
> > fastdebug/support/native/jdk.jdwp.agent/libjdwp/linker_md.o] Error 1
> > gmake[2]: *** [jdk.jdwp.agent-libs] Error 1
> > gmake[2]: *** Waiting for unfinished jobs....
> > 
> > ERROR: Build failed for target 'images' in configuration 'linux-
> > x86_64-server-fastdebug' (exit code 2) 
> > 
> > 
> > 
> > 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
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/20190909/59171fe3/attachment-0001.html>

More information about the serviceability-dev mailing list