RFR: JDK-8227021: VM fails if any sun.boot.library.path paths are longer than JVM_MAXPATHLEN

David Holmes david.holmes at oracle.com
Thu Jul 4 21:21:59 UTC 2019 

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?

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
-----

> 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

More information about the hotspot-dev mailing list