RFR(s): 8171508: Multiple issues with os::jvm_path

[David Holmes]

On 22/12/2016 4:27 PM, Thomas Stüfe wrote:
> Hi David,
>
> On Thu, Dec 22, 2016 at 1:56 AM, David Holmes <david.holmes at oracle.com
> <mailto:david.holmes at oracle.com>> wrote:
>
>     Hi Thomas,
>
>     I think this all seems reasonable. Unfortunately I can't see this as
>     anything higher than the P4 bug it is already categorised as, and as
>     such we are out of time to push this unless we get a second review
>     right now so this can be pushed by the Dec 21 deadline.
>
>
> No problem. As I wrote, I leave the severity to Oracle. We can also do
> it in 10 and backport to 9.

Ok.

> I honestly do not fully understand the story behind -XXaltjvm, I did not
> find any documentation and the source comments are sparse. But I see
> that it did catch quite a lot of bad PR as a security hole (googling
> XXaltjvm + exploit gives quite some hits). There are even suggestions in
> the net that this option was intended as a deliberate backdoor :) I do
> not understand why Oracle does not strip the option out of release
> builds, but as I wrote I do not know the story behind this option fully.

Before my time, but as I understand it, it was for ease of 
testing/debugging by allowing you to point to a different JVM without 
needing to "install" it into a location you may not have access to (eg. 
/usr/java/...). You could send customers a debug VM, or specially 
modified product VM and have them run it in their product environment to 
collect additional data etc., just by changing how it was launched.

As for security ... I think that it is generally understood that if you 
can control the command-line then you can do whatever you want (eg 
-Xbootclasspath).

> As for us at SAP, I'll talk this over with the guys at work and on the
> SAP JVM we might just remove this option from the release builds
> altogether. Unfortunately this would mean the gtests would not run on a
> release VM. I see this not as such a big deal however because gtests on
> release VM are of limited use anyway when you miss all asserts.

True.

>
>     Does it prevent the AIX port for -XXaltjvm?
>
>
> Not really. I would have preferred to fix this first in the other OSes
> and port it to AIX, but I can do the fixed version in AIX first. Then we
> would be better than other ports :)
>
> I started vacation now, so I won't be able to do the AIX changes before
> next year anyway.

Okay. Enjoy your vacation. I'll be away part of January myself.

Cheers,
David
-----

>
>
>     Thanks,
>     David
>
>
> Thank you!
>
> Thomas
>
>
>
>     On 22/12/2016 1:20 AM, Thomas Stüfe wrote:
>
>         Hi all,
>
>         I would like your reviews for the following issue:
>
>         issue: https://bugs.openjdk.java.net/browse/JDK-8171508
>         <https://bugs.openjdk.java.net/browse/JDK-8171508>
>         webrev:
>         http://cr.openjdk.java.net/~stuefe/webrevs/8171508-multiple-issues-with-os_jvmpath/webrev.00/webrev/index.html
>         <http://cr.openjdk.java.net/~stuefe/webrevs/8171508-multiple-issues-with-os_jvmpath/webrev.00/webrev/index.html>
>
>         I found that we had forgotten to port -XXaltjvm handling to AIX
>         (needed for
>         gtests). -XXaltjvm is used to run the jvm with a different
>         hotspot. The
>         option gets used in the gtestLauncher to hand it a jdk located
>         separately
>         from gtest libjvm.
>
>         When looking at the code a number of issues came up and I
>         decided to fix
>         them first before porting the code to AIX.
>
>         The main issue I found is that realpath() was used in an unsafe
>         way. Before
>         POSIX.1-2008, realpath() was broken by design, because there was
>         no safe
>         way to prevent the output buffer from overflowing. The function
>         returns a
>         path, but there is no sure way to predict the maximum path length.
>
>         POSIX1.2008 fixed this by specifying that if the output buffer
>         argument was
>         NULL, the function would return the path in dynamically
>         allocated memory
>         which the user would have to free().
>
>         Before POSIX1.2008, if the output buffer argument was NULL, the
>         behaviour
>         was left up to the implementators by POSIX; I believe that on
>         Linux and
>         BSD, the behaviour was always to return dynamically allocated
>         memory.
>
>         I hope it is safe to assume that the POSIX1.2008 behaviour is
>         the standard
>         now for our Posix platforms. My fix wraps realpath() with a new
>         function
>         os::Posix::realpath() which uses the new Posix behaviour.
>
>         See also:
>         Pre POSIX1.2008:
>         http://pubs.opengroup.org/onlinepubs/009695399/functions/realpath.html
>         <http://pubs.opengroup.org/onlinepubs/009695399/functions/realpath.html>
>         POSIX1.2008:
>         http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html
>         <http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html>
>         (see Rationale section below)
>
>
>         Other issues this change fixes:
>
>         - There is a logic trying to determine if the path of the loaded
>         libjvm.so
>         points to a valid JDK image by searching for "/jre/lib". That
>         logic was
>         broken after JDK-8066474 "Remove the lib/$ARCH directory from
>         Linux and
>         Solaris images".
>         - At some places return codes of realpath() were not checked.
>         - At other places error conditions were asserted, but not
>         handled in the
>         release case.
>
>         Please note that I tried to keep the change minimal. The code
>         would benefit
>         from a cleanup in a future release. I only fixed obvious issues, and
>         refrained from any unnecessary changes.
>
>         I built the changes on Solaris, Linux and MacOS. I tested on all
>         platforms
>         simply by letting the gtestLauncher run, which executes the
>         -XXaltjvm code
>         path.
>
>         The realpath issue may be security related, but the severity
>         depends on how
>         large PATH_MAX is on the build platform compared to the maximum
>         possible
>         path length one would see on filesystems at target systems. I
>         would like to
>         see this fixed in jdk 9 but that decision is up to Oracle.
>
>         Thanks & Kind Regards, Thomas
>
>