RFR: 8229378: jdwp library loader in linker_md.c quietly truncates on buffer overflow
Adam Farley8
adam.farley at uk.ibm.com
Wed Sep 11 11:18:30 UTC 2019
Hi Serguei,
If you're happy with the fix, then here's a webrev without the os.cpp bit.
http://cr.openjdk.java.net/~afarley/8229378.3/webrev/
That was only included to make direct testing possible, and I want to make
sure it doesn't get included by accident.
Best Regards
Adam Farley
IBM Runtimes
"serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com> wrote on
11/09/2019 08:13:47:
> 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: 11/09/2019 08:14
> Subject: Re: RFR: 8229378: jdwp library loader in linker_md.c
> quietly truncates on buffer overflow
>
> Hi Adam,
>
> I'm Okay with this fix.
> If nobody else have comments then I'll build it, test a little bit
> and push to the jdk/jdk repo.
>
> Thanks,
> Serguei
>
>
> On 9/9/19 09:53, Adam Farley8 wrote:
> 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
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/20190911/5603fe19/attachment-0001.html>
More information about the serviceability-dev
mailing list