On Sat, 8 Jan 2022 09:11:05 GMT, Daniel Jelinski <d...@openjdk.java.net> wrote:
>> Clean up of various issues related to error handling and memory management
>
> 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
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
-------------
PR: https://git.openjdk.java.net/jdk/pull/6090
