RFR: 8275640 (win) java.net.NetworkInterface issues with IPv6-only environments [v3]

[Daniel Jelinski]

On Sat, 8 Jan 2022 19:00:52 GMT, Mark Sheppard <msheppar at openjdk.org> wrote:

>> Daniel Jelinski has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address problems reported by clang-tidy
>
> src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 271:
> 
>> 269:     curr = *netifPP;
>> 270:     ret = lookupIPAddrTable(env, &tableP);
>> 271:     if (ret == -1) {
> 
> on the face of it, this change looks reasonable as one might think "let the processing continue for IPv6", however it introduces some problems in subsequent processing. Most notably, it assigns a NULL value to tableP , which is then fed to the function enumAddresses_win_ipaddrtable and this performs a sanity check for tableP == NULL and returns 0. Also enumAddresses_win_ipaddrtable takes the inout parameter netaddr *netaddrP; which is a local variable (stack variable) which hasn't been properly initialized and only has a valid value assigned for a successful return from enumAddresses_win_ipaddrtable. Thus, this inout varaiable has indeterminates values (i.e. what ever is on the stack) which are then assigned in the else block (for the return 0 case)
>  else{
>             curr->addrs = netaddrP;
>             curr->naddrs += ret;
>             curr = curr->next;
>         }
> and so subsequent indeterminate processing and potental crash is significant as the netif data structures spurious pointer assignments
> 
> I think the current coded return policy is good, but should probably include *netifPP = NULL as nothing is being returned.
> 
> The function getAllInterfacesAndAddresses also inconsistently handles the out value of *netifPP as it is not always assigned a value in enumAddresses_win_ipaddrtable. As such the addition of call to free_netif  on this pointer may lead to indeterminate behavious -- Thus the assignment and update of netifPP needs to be addressed

I modified all functions to return sensible pointer values (possibly null) in all success (nonnegative) return cases. Returned pointers may contain nonsense only in failure cases. In particular, `enumAddresses_win_ipaddrtable` was fixed to set `*netaddrPP = NULL;` in 784d5fb332aee4215c02751636c848ffbb0da2b9.

> src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 282:
> 
>> 280:     }
>> 281:     while (curr != NULL) {
>> 282:         netaddr *netaddrP;
> 
> netaddr *netaddrP = NULL;  just in case enumAddresses_win_ipaddrtable does not update it for an error return

netaddrP is always updated when `enumAddresses_win_ipaddrtable` returns a nonnegative value

> src/java.base/windows/native/libnet/NetworkInterface_winXP.c line 285:
> 
>> 283:         ret = enumAddresses_win_ipaddrtable(env, curr, &netaddrP, tableP);
>> 284:         if (ret < 0) {
>> 285:             free_netif(*netifPP);
> 
> seems like the right thing to do, but *netifPP may not be properly assigned a valid pointer

`netifPP` is always assigned by `enumInterfaces` in nonnegative return cases. If `enumInterfaces` returns a negative value, this function (`getAllInterfacesAndAddresses`) aborts much earlier.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6090