RFR: JDK-8227021: VM fails if any sun.boot.library.path paths are longer than JVM_MAXPATHLEN
Adam Farley8
adam.farley at uk.ibm.com
Mon Jul 8 12:45:28 UTC 2019
Hi David,
David Holmes <david.holmes at oracle.com> wrote on 04/07/2019 22:21:59:
> From: David Holmes <david.holmes at oracle.com>
> To: Adam Farley8 <adam.farley at uk.ibm.com>
> Cc: hotspot-dev at openjdk.java.net
> Date: 04/07/2019 22:22
> Subject: Re: RFR: JDK-8227021: VM fails if any sun.boot.library.path
> paths are longer than JVM_MAXPATHLEN
>
> Hi Adam,
>
> On 5/07/2019 2:41 am, Adam Farley8 wrote:
> > Hi David,
> >
> > To detect a too-long path when it's being passed in, the best option
> > I can see is to check it in two places:
>
> Right, but my outstanding question relates to the existing code today.
> Where will we detect that a path element is too long?
Ahh, right.
Right now, the place checking the length of the paths is split_path,
the method I modified in oc.cpp. Specifically this bit:
----
if (len > JVM_MAXPATHLEN) {
return NULL;
}
----
This seemed wrong to me at the time, because it meant that is *any* of
the paths was too long, the method splitting up the paths string would
return null, and fail to check any of the library locations.
If we agree that the correct behaviour would be to throw an error and
kill the vm if any of the paths is too long, then the simplest option
would be to replace the "return NULL" in that "if" with an error.
Something like this, perhaps?
---
if (len > JVM_MAXPATHLEN) {
vm_exit_during_initialization("java.lang.VirtualMachineError",
"One or more of the
sun.boot.library.path "
"paths has exceeded the maximum path
length "
"for this system.");
}
---
With fewer new-lines, of course. ;)
>
> I'm still not sure whether the VM has the right to dictate behaviour
> here or whether this belongs to core-libs. And we need to be very
> careful about any change in behaviour.
>
> > 1) when it's being set initially with the location of libjvm.so,
either:
> > a)in hotspot/os/[os name]/os_[os name].cpp, right before the call
> > to Arguments::set_dll_dir
> > or b), in the Arguments::set_dll_dirfunction itself (ideally the
> > latter)
> >
> > 2) when/if the extra paths are being passed in as a parameter, as they
> > pass through hotspot/share/runtime/arguments.cpp, right after the
line:
> >
> > ---
> > else if (_strcmp_(key, "sun.boot.library.path") == 0)");
> > ---
> >
> > You're right in that this could slow down startup a little, with
> > the length checking, and the potential looping over the -D value
> > to check the length of each path. Not a major slowdown though.
>
> I'm sure Claes would disagree :)
>
> Apologies in advance as I'm about to disappear for two weeks vacation.
>
> David
> -----
No worries. :)
- Adam
>
> > Best Regards
> >
> > Adam Farley
> > IBM Runtimes
> >
> >
> > David Holmes <david.holmes at oracle.com> wrote on 04/07/2019 07:57:14:
> >
> >> From: David Holmes <david.holmes at oracle.com>
> >> To: Adam Farley8 <adam.farley at uk.ibm.com>
> >> Cc: hotspot-dev at openjdk.java.net
> >> Date: 04/07/2019 07:58
> >> Subject: Re: RFR: JDK-8227021: VM fails if any sun.boot.library.path
> >> paths are longer than JVM_MAXPATHLEN
> >>
> >> Hi Adam,
> >>
> >> On 4/07/2019 1:42 am, Adam Farley8 wrote:
> >> > Hi David,
> >> >
> >> > I figured it should be elaborate so we can avoid killing the VM
> >> > if we don't have to.
> >> >
> >> > Ultimately, if we have a list of three paths and the last two
> >> > are invalid, does it matter so long as all the libraries we need
> >> > are in the first path?
> >>
> >> I prefer not see the users error ignored if we can reasonably detect
it.
> >> They set the paths for a reason, and if they paths are invalid they
> >> probably would like to know.
> >>
> >> > As to your question "is it in hostpot or JDK code", I presume you
> >> > mean in the change set. I'm primarily referring to the hotspot
code.
> >>
> >> No I mean where in the current code will we detect that one of these
> >> path elements is too long?
> >>
> >> > Also, if we end up adopting a "kill the vm if any path is too
long"
> >> > approach, we still need to change the JDK code, as those currently
> >> > seem to want to fail if the total length of the
sub.boot.library.path
> >> > property is longer than the maximum length of a single path.
> >> >
> >> > So if you pass in three 100 character paths on Windows, it'll fail
> >> > because they add up to more than the 260 character path limit.
> >>
> >> That seems like a separate bug that should be addressed. :(
> >>
> >> Thanks,
> >> David
> >>
> >> > Best Regards
> >> >
> >> > Adam Farley
> >> > IBM Runtimes
> >> >
> >> >
> >> > David Holmes <david.holmes at oracle.com> wrote on 03/07/2019
08:36:36:
> >> >
> >> >> From: David Holmes <david.holmes at oracle.com>
> >> >> To: Adam Farley8 <adam.farley at uk.ibm.com>
> >> >> Cc: hotspot-dev at openjdk.java.net
> >> >> Date: 03/07/2019 08:36
> >> >> Subject: Re: RFR: JDK-8227021: VM fails if any
sun.boot.library.path
> >> >> paths are longer than JVM_MAXPATHLEN
> >> >>
> >> >> On 2/07/2019 7:44 pm, Adam Farley8 wrote:
> >> >> > Hi David,
> >> >> >
> >> >> > Thanks for your thoughts.
> >> >> >
> >> >> > The user should absolutely have immediate feedback, yes, and I
agree
> >> >> > that "skipping" paths could lead to us loading the wrong
library.
> >> >> >
> >> >> > Perhaps a compromise? We fire off a stderr warning if any
> of the paths
> >> >> > are too long (without killing the VM), we ignore any path
*after*
> >> >> > (and including) the first too-long path, and we kill the VM if
the
> >> >> > first path is too long.
> >> >>
> >> >> My first though is why be so elaborate and not just fail
immediately:
> >> >>
> >> >> Error occurred during initialization of VM
> >> >> One or more sun.boot.library.path elements is too long for this
system.
> >> >> ---
> >> >>
> >> >> ? But AFAICS we don't do any sanity checking of the those
> paths so this
> >> >> would have an impact on startup.
> >> >>
> >> >> I can't locate where we would detect the too-long path
> element, is it in
> >> >> hostpot or JDK code?
> >> >>
> >> >> Thanks,
> >> >> David
> >> >> -----
> >> >>
> >> >> > Warning message example:
> >> >> >
> >> >> > ----
> >> >> > Warning: One or more sun.boot.library.path paths were too long
> >> >> > for this system, and it (along with all subsequent paths) have
been
> >> >> > ignored.
> >> >> > ----
> >> >> >
> >> >> > Another addition could be to check the path lengths for the
property
> >> >> > sooner, thus aborting the VM faster if the default path is too
long.
> >> >> >
> >> >> > Assuming we posit that the VM will always need to load
libraries.
> >> >> >
> >> >> > Best Regards
> >> >> >
> >> >> > Adam Farley
> >> >> > IBM Runtimes
> >> >> >
> >> >> >
> >> >> > David Holmes <david.holmes at oracle.com> wrote on 01/07/2019
22:10:45:
> >> >> >
> >> >> >> From: David Holmes <david.holmes at oracle.com>
> >> >> >> To: Adam Farley8 <adam.farley at uk.ibm.com>, hotspot-
> dev at openjdk.java.net
> >> >> >> Date: 01/07/2019 22:12
> >> >> >> Subject: Re: RFR: JDK-8227021: VM fails if any
> sun.boot.library.path
> >> >> >> paths are longer than JVM_MAXPATHLEN
> >> >> >>
> >> >> >> Hi Adam,
> >> >> >>
> >> >> >> On 1/07/2019 10:27 pm, Adam Farley8 wrote:
> >> >> >> > Hi All,
> >> >> >> >
> >> >> >> > The title say it all.
> >> >> >> >
> >> >> >> > If you pass in a value for sun.boot.library.path consisting
> >> >> >> > of one or more paths that are too long, then the vm will
> >> >> >> > fail to start because it can't load one of the libraries it
> >> >> >> > needs (the zip library), despite the fact that the VM
> >> >> >> > automatically prepends the default library path to the
> >> >> >> > sun.boot.library.path property, using the correct separator
> >> >> >> > to divide it from the user-specified path.
> >> >> >> >
> >> >> >> > So we've got the right path, in the right place, at the
> >> >> >> > right time, we just can't *use* it.
> >> >> >> >
> >> >> >> > I've fixed this by changing the relevant os.cpp code to
> >> >> >> > ignore paths that are too long, and to attempt to locate
> >> >> >> > the needed library on the other paths (if any are valid).
> >> >> >>
> >> >> >> As I just added to the bug report I have a different view
> of "correct"
> >> >> >> here. If you just ignore the long path and keep processing
> other short
> >> >> >> paths you may find the wrong library. There is a user
> error here and
> >> >> >> that error should be reported ASAP and in a way that leads
> to failure
> >> >> >> ASAP. Perhaps we should be more aggressive in aborting the
> VMwhen this
> >> >> >> is detected?
> >> >> >>
> >> >> >> David
> >> >> >> -----
> >> >> >>
> >> >> >> > I've also added functionality to handle the edge case of
> >> >> >> > paths that are neeeeeeearly too long, only for a
> >> >> >> > sub-path (or file name) to push us over the limit *after*
> >> >> >> > the split_path function is done assessing the path length.
> >> >> >> >
> >> >> >> > I've also changed the code we're overriding, on the
assumption
> >> >> >> > that someone's still using it somewhere.
> >> >> >> >
> >> >> >> > Bug: https://urldefense.proofpoint.com/v2/url?
> >> >> >>
> >> >>
> >>
>
u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8227021&d=DwICaQ&c=jf_iaSHvJObTbx-
> >> >> >> siA1ZOg&r=P5m8KWUXJf-
> >> >> >>
> >> >>
> >>
>
CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=cSTGBGkEsu5yl0haJ6it9egPSgixg7mRei6lBDB5Y3k&s=xZzQCnv68xd9hJyyK1obSim38eWSRmLPfuR__9ddZWg&e=
> >> >> >> > Webrev: https://urldefense.proofpoint.com/v2/url?
> >> >> >>
> >> >>
> >>
>
u=http-3A__cr.openjdk.java.net_-7Eafarley_8227021_webrev_&d=DwICaQ&c=jf_iaSHvJObTbx-
> >> >> >> siA1ZOg&r=P5m8KWUXJf-
> >> >> >>
> >> >>
> >>
>
CeVJc0hDGD9AQ2LkcXDC0PMV9ntVw5Ho&m=cSTGBGkEsu5yl0haJ6it9egPSgixg7mRei6lBDB5Y3k&s=-
> >> >> >> hKU0zUd_0LDT08wTilexgI54EeSgt8xUk97i6V63Bk&e=
> >> >> >> >
> >> >> >> > Thoughts and impressions welcome.
> >> >> >> >
> >> >> >> > 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
More information about the hotspot-dev
mailing list