ThreadLocalRandom clinit troubles

[Peter Levart]

On 06/19/2014 08:04 PM, Martin Buchholz wrote:
> On Thu, Jun 19, 2014 at 1:22 AM, Alan Bateman <Alan.Bateman at oracle.com>
> wrote:
>
>> On 19/06/2014 05:25, Martin Buchholz wrote:
>>
>>> ThreadLocalRandom's clinit method creates an intermediate broken state of
>>> ThreadLocalRandom and then proceeds to run some networking code to get
>>> some
>>> more machine-specific entropy in initialSeed().  This will fail if the
>>> networking code ever recursively uses a (not yet functional)
>>> ThreadLocalRandom.  The clinit for InetAddress can cause arbitrary code to
>>> be run,
>>>
>>> at
>>> java.util.ServiceLoader$LazyIterator.hasNextService(
>>> ServiceLoader.java:354)
>>> at java.util.ServiceLoader$LazyIterator.hasNext(ServiceLoader.java:393)
>>> at java.util.ServiceLoader$1.hasNext(ServiceLoader.java:474)
>>> at java.net.InetAddress$3.run(InetAddress.java:923)
>>> at java.net.InetAddress$3.run(InetAddress.java:918)
>>> at java.security.AccessController.doPrivileged(Native Method)
>>> at java.net.InetAddress.createNSProvider(InetAddress.java:917)
>>> at java.net.InetAddress.<clinit>(InetAddress.java:962)
>>>
>>> if the sun.net.spi.nameservice.provider system property is defined.
>>>
>>> The current strategy of ThreadLocalRandom relying on other java code for
>>> initialization seems risky.
>>>
>> Using a name service provider other than the default is going to interact
>> badly here. So is this configured to use the JNDI-DNS provider ("dns,sun")
>> or something else? This provider mechanism has been a source of many
>> problems, recursive initialization and stack overflow mostly because any
>> custom provider is likely going to use the network and resolve host names.
>> It can interact very badly with security when the provider doesn't have
>> AllPermision because attempts to establish connections involve security
>> checks that often need to do lookups too.
>>
>> So I'm curious if there is more to this stack trace to put more context on
>> the issue.
>
> The way we are actually seeing this is:
> - there are jdk8 jtreg tests that set the sun.net.spi.nameservice.provider
> property.  Grepping:
>
> ./java/net/Inet4Address/textToNumericFormat.java
> ./java/net/URLPermission/nstest/lookup.sh
> ./sun/net/InetAddress/nameservice/dns/cname.sh
> ./sun/net/InetAddress/nameservice/deadlock/Hang.java
> ./sun/net/InetAddress/nameservice/chaining/Providers.java
> ./sun/net/InetAddress/nameservice/simple/DefaultCaching.java
> ./sun/net/InetAddress/nameservice/simple/CacheTest.java
> ./sun/security/krb5/auto/KDC.java
> ./sun/security/krb5/canonicalize/Test.java
>
> - we have local modifications to classloading that happen to use TLR
> via ConcurrentSkipListMap - a "reasonable" thing to do.  Probably the
> simplest way to provoke a failure is to try to use a TLR from e.g.
> URLClassPath.
>
> In general, any core library boot class should try hard to avoid
> loading/invoking code from the user's classpath.

Hi Martin,

Does my proposed patch solve these issues or does the 
"sun.net.spi.nameservice.provider" triggered changed initialization 
order provoke other failures. I can imagine there might be other issues. 
Perhaps a more lazy initialization of NetworkInterface class that does 
not trigger initialization of NS providers could help. We just need to 
invoke two methods on NetworkInterface:

- static NetworkInterface.getNetworkInterfaces()
- instance NetworkInterface.getHardwareAddress()

both of which could provide the result without the need of NS providers, 
I think.

This would solve the most general case of using TLR. The case that 
doesn't involve SecureRandom's help which I think is rarely needed and 
not default.


Regards, Peter

>
>
>> It may be another example to back a suggestion to just drop the
>> JDK-internal name service provider mechanism. But in general, I think you
>> are right, it's not good for TLR initialization to trigger arbitrary code
>> to execute.
>>
>> -Alan.
>>