Haikuports: dev-lang/openjdk NetworkInterface.isLoopback() crashes with java.net.SocketException

[volo-droid]

Description

NetworkInterface methods that call native code (e.g. isLoopback(), isUp(), getMTU()) crash with an exception.

Original issue

Gradle build tool cannot be run on Haiku's OpenJDK. Upon startup Gradle 6.x searches for a loopback interface and fails with a crash:

java.net.SocketException: Invalid Argument (getFlags() failed)
        at java.net.NetworkInterface.isLoopback0(Native Method)
        at java.net.NetworkInterface.isLoopback(NetworkInterface.java:411)
        at org.gradle.internal.remote.internal.inet.InetAddresses.analyzeNetworkInterface(InetAddresses.java:55)
        at org.gradle.internal.remote.internal.inet.InetAddresses.analyzeNetworkInterfaces(InetAddresses.java:47)
        at org.gradle.internal.remote.internal.inet.InetAddresses.<init>(InetAddresses.java:40)
        at org.gradle.internal.remote.internal.inet.InetAddressFactory.init(InetAddressFactory.java:99)
        at org.gradle.internal.remote.internal.inet.InetAddressFactory.getWildcardBindingAddress(InetAddressFactory.java:84)
        at org.gradle.cache.internal.locklistener.FileLockCommunicator.<init>(FileLockCommunicator.java:49)
        at org.gradle.cache.internal.locklistener.DefaultFileLockContentionHandler.getCommunicator(DefaultFileLockContentionHandler.java:263)
        at org.gradle.cache.internal.locklistener.DefaultFileLockContentionHandler.reservePort(DefaultFileLockContentionHandler.java:255)
        at org.gradle.cache.internal.DefaultFileLockManager.lock(DefaultFileLockManager.java:108)
…

Sample to reproduce

import java.net.InetAddress;
import java.net.NetworkInterface;
import java.net.SocketException;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;

public class Net {

    public static void main(String[] args) throws Exception {
        Net net = new Net();
        net.analyze();
    }

    public void analyze() throws Exception {
        Enumeration<NetworkInterface> interfaces = NetworkInterface.getNetworkInterfaces();
        if (interfaces == null) {
            log("No network interfaces found");
            return;
        }

        while (interfaces.hasMoreElements()) {
            NetworkInterface networkInterface = interfaces.nextElement();
            log("Network interface %s", networkInterface.getDisplayName());

            try {
                //  NetworkInterface.isLoopback() throws 
                //    "java.net.SocketException: Invalid Argument (getFlags() failed)"
                log("Is this a loopback interface? %s", networkInterface.isLoopback());

                //  NetworkInterface.isUp() also throws the same exception
                log("Is this a interface up? %s", networkInterface.isUp());
            } catch (Throwable e) {
                log("Error while querying interface %s", networkInterface);
                e.printStackTrace();
            }
        }
    }

    private void log(String message, Object... args) {
        System.out.println(String.format(message, args));
    }    
}

When that code is run on my Haiku installation, it outputs the following exceptions:

Network interface loop
Error while querying interface name:loop (loop)
java.net.SocketException: Invalid Argument (getFlags() failed)
        at java.base/java.net.NetworkInterface.isLoopback0(Native Method)
        at java.base/java.net.NetworkInterface.isLoopback(NetworkInterface.java:458)
        at Net.analyze(Net.java:28)
        at Net.main(Net.java:12)
Network interface /dev/net/pcnet/0
Error while querying interface name:/dev/net/pcnet/0 (/dev/net/pcnet/0)
java.net.SocketException: Invalid Argument (getFlags() failed)
        at java.base/java.net.NetworkInterface.isLoopback0(Native Method)
        at java.base/java.net.NetworkInterface.isLoopback(NetworkInterface.java:458)
        at Net.analyze(Net.java:28)
        at Net.main(Net.java:12)

Environment

Haiku x86_64
Walter (Revision hrev 54672)
Kernel
23.10.2020 6:34:17

VM
VMWare Fusion 12 on macOS 10.15.7
2 CPUs, 8192 Mb

OpenJDK
Reproduces on various versions: 1.8.u242_b8-2, 11.0.4.11-1, 13.0.2.8-1

[volo-droid]

I've found the root cause. OpenJDK performs an ioctl call to query network interface flags:
https://github.com/korli/haiku-jdk11u/blob/ca952635c894ad92e3a7a3b6e4eec6976a960f8a/src/java.base/unix/native/libnet/NetworkInterface.c#L1331

Contrary to other Unix systems (e.g.: Linux, AIX), Haiku expects a size of ifreq struct to be passed as the last parameter of ioctl call. The length value seems to be not actually used for anything besides checking that it's there and is not less then 32 (IF_NAMESIZE). If that check fails ioctl call returns -1 which would be transformed into Java world exception.

To fix the issue all ioctl calls in OpenJDK NetworkInterface.c should be conditionally compiled for Haiku call convention (optionally defining a macro for that call):

#ifdef __HAIKU__
int result = ioctl(sock, SIOCGIFFLAGS, (char *)&if2, sizeof(struct ifreq));
#else
int result = ioctl(sock, SIOCGIFFLAGS, (char *)&if2);
#endif

IMHO the best would be to align ioctl calling convention with other Unix systems and remove a requirement of passing the size of ifreq structure. But that's probably quite a big change to the system itself.

[volo-droid]

Actually there's already one big #ifdef __HAIKU__ block, so ioctl calls can be updated just inside of it.

[diversys]

Could you create a PR?

[volo-droid]

@diversys yes, sure. Should I create it in korli's OpenJDK repositories (e.g. https://github.com/korli/haiku-jdk11u) ?

[diversys]

Yup.

[korli]

I pushed a change at Haiku Gerrit: https://review.haiku-os.org/c/haiku/+/3360
This is just annoying, as ioctl with 4 arguments won't ever be upstreamed.

[volo-droid]

@korli Can you please explain what does that change do? If I get it right and it translates ioctl(fd, op, arg) to ioctl(fd, op, arg, 0) then it won't help us with this OpenJDK issue, because:

The length value seems to be not actually used for anything besides checking that it's there and is not less then 32 (IF_NAMESIZE). If that check fails ioctl call returns -1 which would be transformed into Java world exception.

If I'm not wrong, that's the check:
https://github.com/haiku/haiku/blob/abb59d7351c7ddb50c63c40430a82d94fa61917a/src/add-ons/kernel/network/stack/datalink.cpp#L323

// We also accept partial ifreqs as long as the name is complete.
if (*_length < IF_NAMESIZE)
    return B_BAD_VALUE;

Also there're couple more similar checks in that file, for example:

if (*_length < sizeof(struct ifaliasreq))
    return B_BAD_VALUE;`

So, to make ioctl calls from openjdk succeed, we would need to remove those checks in the driver code, or make ioctl(fd, op, arg) be equal to ioctl(fd, op, arg, sizeof(arg)), which AFAIR is not POSIX-compliant.

---

On my side, I've setup haikurporter and build openjdk8 locally patching the NetworkInterface.c file. Now NetworkInterface.java calls work as expected without exceptions. I can go through the whole openjdk code and patch all ioctl invocations with ifdef __HAIKU__, there're not so many of them so that should not be a big deal. Still, I'd be up to having a centralized ioctl solution, where we can just call any ioctl call without the size_t parameter and it would work, aligning Haiku with Linux/Unix behavior.

[korli]

@volo-droid after a closer analysis, the ioctl calls don't result in a crash here. A NULL check was missing in getMacAddress: https://github.com/korli/haiku-jdk11u/commit/6cd3c03472d23368a5893490dac47c74eeba5ca9

[volo-droid]

@korli That must be a different bug. Have a look at my example, the crash happens either on NetworkInterface.isLoopback(), NetworkInterface.isUp() or NetwokInterface.getMTU() invocations. All of those calls immediately delegates work to the corresponding native methods: isLoopback0, isUp0, getMTU0, and none of those methods actually call getMacAddress(). Instead isLoopback0 and isUp0 calls getFlags0() which in turn delegates the actual work to getFlags(). Haiku's version of that last one looks like:

static int getFlags(int sock, const char *ifname, int *flags) {
    struct ifreq if2;
    memset((char *)&if2, 0, sizeof(if2));
    strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name) - 1);

    if (ioctl(sock, SIOCGIFFLAGS, (char *)&if2) < 0) {
        return -1;
    }

    if (sizeof(if2.ifr_flags) == sizeof(short)) {
        *flags = (if2.ifr_flags & 0xffff);
    } else {
        *flags = if2.ifr_flags;
    }
    return 0;
}

The call ioctl(sock, SIOCGIFFLAGS, (char *)&if2) there returns -1. I've tried to call ioctl the same way from the pure C app and it constantly returns -1, unless I specify the fourth parameter as either sizeof(struct ifreq) or any integer >=32 (IF_NAMESIZE constant in datalink.cpp).

NetworkInterface.getMTU() has a bit different call chain: getMTU0() -> getMTU(). But in the end it does a similar ioctl call:

    if (ioctl(sock, SIOCGIFMTU, (char *)&if2) < 0) {
        JNU_ThrowByNameWithMessageAndLastError
            (env, JNU_JAVANETPKG "SocketException", "ioctl(SIOCGIFMTU) failed");
        return -1;
    }

The only Java method that results in getMacAddress call is actually NetworkInterface.getMacAddr(). But yes, it missed the NULL check there 😉

[volo-droid]

Also, as I mentioned above, I had sucessfully patched ioctl calls in NetworkInterface.c file in my local haikuports tree and rebuilt the OpenJDK. Now the sample from my report produces the following output:

▸ java Net

Network interface /dev/net/pcnet/0
MTU? 1500
Is this a loopback interface? false
Is this a interface up? true
Network interface loop
MTU? 16384
Is this a loopback interface? true
Is this a interface up? true

[korli]

@volo-droid indeed sorry for the mix-up
The Haiku change is needed because the OpenJDK code should work as is. Now yes, we need a workaround for the current Haiku release. Probably at Haikuports.

[volo-droid]

The Haiku change is needed because the OpenJDK code should work as is. Now yes, we need a workaround for the current Haiku release. Probably at Haikuports.

@korli Does that mean, I can proceed with creating a PR to update ioctl calls in openjdk? Or you have a better idea to somehow apply patches through haikuports recipe? In the second case I think I can still create a PR which you'd review and close later, but use it as a source for a patch file.

[korli]

a Haikuports patch would be better in this case

[korli]

I added a patch in 0c53275807ec4873d69691f30c0482773237909e

[korli]

all versions are rebuilt with the patch.

[volo-droid]

@korli AFAIU you're gonna remove those patches once https://review.haiku-os.org/c/haiku/+/3360 is merged and datalink.cpp is updated to remove length checks, right? Regarding the latter, I don't really get what is passed right now as the value of _length parameter here, in case ioctl has been called just with 3 parameters. Is it undefined and we can expect any kind of garbage there?

This is just annoying, as ioctl with 4 arguments won't ever be upstreamed.

Is there a defined plan or roadmap of upstreaming the Haiku-specific changes to OpenJDK repository? Also I haven't found the info on the forum, so maybe you could tell me what's happened to the OpenJDK Haiku Port project initiative and how it relates to the current state of OpenJDK on Haiku (forked AdoptOpenJDK repositories)?

[korli]

@korli AFAIU you're gonna remove those patches once https://review.haiku-os.org/c/haiku/+/3360 is merged and datalink.cpp is updated to remove length checks, right? Regarding the latter, I don't really get what is passed right now as the value of _length parameter here, in case ioctl has been called just with 3 parameters. Is it undefined and we can expect any kind of garbage there?

The patches will stay at least until beta3 (buildbots build with beta2 at the moment).
the value of the lengh parameter is undefined, actually it's up to the driver using the ioctl buffer and length parameter. Given the ioctl code is not specific, the network stack shouldn't assume anything about the 4th parameter. Now, we don't want to break user relying on partial requests. We could eventually drop support for partial requests, and be fine with it (hopefully we'll get to this).

Is there a defined plan or roadmap of upstreaming the Haiku-specific changes to OpenJDK repository? Also I haven't found the info on the forum, so maybe you could tell me what's happened to the OpenJDK Haiku Port project initiative and how it relates to the current state of OpenJDK on Haiku (forked AdoptOpenJDK repositories)?

No, no defined plan. Upstreaming to OpenJDK doesn't get us much. FreeBSD does about the same ATM.
I think AdoptOpenJDK proposed to the BSD Port to upstream to them, I don't know what results from this proposal.