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

[David Holmes]

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.

Does it prevent the AIX port for -XXaltjvm?

Thanks,
David

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
> webrev:
> 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
> POSIX1.2008:
> 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
>