[PATCH 0/3] MR10628: nsiproxy.sys: Avoid null pointer dereference on if_nameindex() failure.

[Michał Durak (＠mdurak)]

`if_nameindex()` always fails with EACCES on my Android 15 device under Termux. On failure, `if_nameindex()` returns null and sets errno. The original implementation in Wine didn't check for this, which meant that `entry->if_index` would be dereferencing null and failing with an access violation error. As all calls to `if_list_lock()` are guarded with a mutex lock, the lock would never be released, causing deadlocks.

To reproduce this on my PC, I used the \`LD_PRELOAD\` trick with this dummy dynamic library:

```c
#define _GNU_SOURCE
#include <net/if.h>
#include <errno.h>
#include <stdlib.h>

struct if_nameindex *if_nameindex(void)
{
    errno = EACCES;
    return NULL;
}
```

```console
$ LD_PRELOAD=/home/user/wine/libborked_if_nameindex.so WINEDEBUG=+nsi,+iphlpapi,+seh ./wine ipconfig
0054:trace:nsi:DriverEntry (00007FFFFF219CF0, L"\\Registry\\Machine\\System\\CurrentControlSet\\Services\\nsiproxy")
0054:trace:iphlpapi:GetIfTable2 table 00007FFFFEC0FA00
0054:trace:iphlpapi:GetIfTable2Ex level 0, table 00007FFFFEC0FA00
0054:trace:nsi:NsiAllocateAndGetTable 1 00006FFFFDD254F0 0 00007FFFFEC0F940 8 00007FFFFEC0F948 1092 00007FFFFEC0F950 216 00007FFFFEC0F958 600 00007FFFFEC0F93C 0
0054:trace:nsi:NsiEnumerateObjectsAllParameters 1 0 00006FFFFDD254F0 0 00007FFFFF27F150 8 00007FFFFF27F360 1092 00007FFFFF290470 216 00007FFFFF293A80 600 00007FFFFEC0F83C
0058:trace:nsi:nsi_ioctl ioctl 121000 insize 56 outsize 122628
0058:trace:nsi:ifinfo_enumerate_all 0x7fffff2bafa4 8 0x7fffff2bb1a4 1092 0x7fffff2cc2a4 216 0x7fffff2cf8a4 600 0x7ffffef0f948
0058:trace:seh:handle_syscall_fault code=c0000005 flags=0 addr=0x7fdb07114783 ip=7fdb07114783 tid=0058
0058:trace:seh:handle_syscall_fault  info[0]=0000000000000000
0058:trace:seh:handle_syscall_fault  info[1]=0000000000000000
0058:trace:seh:handle_syscall_fault  rax=0000000000000000 rbx=00007fffff2bb1a4 rcx=0000000000000000 rdx=0000000000000001
0058:trace:seh:handle_syscall_fault  rsi=0000000000000000 rdi=00007fdb0711cf80 rbp=00007fffff2cc2a4 rsp=00007ffffed0e8f0
0058:trace:seh:handle_syscall_fault   r8=0000000000000000  r9=0000000000000000 r10=00007fdb0768a898 r11=00007fdb076a0250
0058:trace:seh:handle_syscall_fault  r12=00007ffffef0f948 r13=00007fffff2bafa4 r14=0000000000000008 r15=0000000000000038
0058:trace:seh:handle_syscall_fault returning to user mode ip=00006ffffde12515 ret=c0000005
0054:trace:nsi:NsiFreeTable 00007FFFFF27F150 00007FFFFF27F360 00007FFFFF290470 00007FFFFF293A80
00a4:err:ntoskrnl:ZwLoadDriver failed to create driver L"\\Registry\\Machine\\System\\CurrentControlSet\\Services\\winebth": c00000e5
003c:fixme:service:scmdatabase_autostart_services Auto-start service L"winebth" failed to start: 1359
00e4:trace:iphlpapi:GetAdaptersAddresses (0, 00000080, 0000000000000000, 00007FFFFE32E280, 00007FFFFE2FFA8C)
00e4:trace:nsi:NsiAllocateAndGetTable 1 00006FFFFDD254F0 0 00007FFFFE2FF9B0 8 00007FFFFE2FF9B8 1092 00007FFFFE2FF9C0 216 00007FFFFE2FF9C8 600 00007FFFFE2FF9AC 0
00e4:trace:nsi:NsiEnumerateObjectsAllParameters 1 0 00006FFFFDD254F0 0 00007FFFFE32F290 8 00007FFFFE8F7DC0 1092 00007FFFFE908ED0 216 00007FFFFE90C4E0 600 00007FFFFE2FF87C
0058:trace:nsi:nsi_ioctl ioctl 121000 insize 56 outsize 122628
0058:trace:nsi:ifinfo_enumerate_all 0x7fffff27f224 8 0x7fffff27f424 1092 0x7fffff290524 216 0x7fffff293b24 600 0x7ffffef0f948 <-- stuck in pthread_mutex_lock( &if_list_lock );
```

The changes to `adapters_addresses_alloc` and `ipv4_neighbour_enumerate_all` deal with the edge case of no network adapters being present/detectable by Wine. On systems where `if_nameindex()` always fails, `add_entry()` in ndis.c would never be called.

On the topic of correctness, MSDN says that `GetAdaptersAddresses` may return `ERROR_NO_DATA` if "No addresses were found for the requested parameters", which sounds right to me, but I wasn't able to come up with an invocation that would give me that return to put in the tests.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10628

[PATCH 1/3] nsiproxy.sys: Avoid null pointer dereference on if_nameindex() failure.

[Michał Durak]

From: Michał Durak<[e-mail]>

---
 dlls/nsiproxy.sys/ndis.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/dlls/nsiproxy.sys/ndis.c b/dlls/nsiproxy.sys/ndis.c
index 8c15186ed53..5dde17244d9 100644
--- a/dlls/nsiproxy.sys/ndis.c
+++ b/dlls/nsiproxy.sys/ndis.c
@@ -29,6 +29,7 @@
 #include <sys/socket.h>
 #include <sys/ioctl.h>
 #include <unistd.h>
+#include <errno.h>
 
 #ifdef HAVE_NET_IF_H
 #include <net/if.h>
@@ -308,9 +309,16 @@ static struct if_entry *add_entry( UINT index, char *name )
 
 static unsigned int update_if_table( void )
 {
-    struct if_nameindex *indices = if_nameindex(), *entry;
+    struct if_nameindex *indices, *entry;
     unsigned int append_count = 0;
 
+    indices = if_nameindex();
+    if (!indices)
+    {
+        ERR( "if_nameindex failed, errno %d.\n", errno );
+        return 0;
+    }
+
     for (entry = indices; entry->if_index; entry++)
     {
         if (!find_entry_from_index( entry->if_index ) && add_entry( entry->if_index, entry->if_name ))
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10628

[PATCH 2/3] nsiproxy.sys: Avoid malloc(0) in case no network interfaces are present.

[Michał Durak]

From: Michał Durak<[e-mail]>

---
 dlls/nsiproxy.sys/ip.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/nsiproxy.sys/ip.c b/dlls/nsiproxy.sys/ip.c
index 2b724db8dae..6c4d5808be4 100644
--- a/dlls/nsiproxy.sys/ip.c
+++ b/dlls/nsiproxy.sys/ip.c
@@ -1228,6 +1228,12 @@ static NTSTATUS ipv4_neighbour_enumerate_all( void *key_data, UINT key_size, voi
                                      NULL, 0, NULL, 0, &iface_count )))
         return status;
 
+    if (!iface_count)
+    {
+        *count = 0;
+        return STATUS_SUCCESS;
+    }
+
     if (!(luid_tbl = malloc( iface_count * sizeof(*luid_tbl) )))
         return STATUS_NO_MEMORY;
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10628

[PATCH 3/3] iphlpapi: Avoid out-of-bounds read if no network interfaces are present.

[Michał Durak]

From: Michał Durak<[e-mail]>

If *count was zero, aa would end up pointing to a zero-size allocation,
leading to out-of-bounds access in unicast_addresses_alloc().
---
 dlls/iphlpapi/iphlpapi_main.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c
index fe2de41c4a3..b315490b1f2 100644
--- a/dlls/iphlpapi/iphlpapi_main.c
+++ b/dlls/iphlpapi/iphlpapi_main.c
@@ -1248,7 +1248,7 @@ static DWORD dns_info_alloc( IP_ADAPTER_ADDRESSES *aa, ULONG family, ULONG flags
 
 static DWORD adapters_addresses_alloc( ULONG family, ULONG flags, IP_ADAPTER_ADDRESSES **info, ULONG *count )
 {
-    IP_ADAPTER_ADDRESSES *aa;
+    IP_ADAPTER_ADDRESSES *aa = NULL;
     NET_LUID *luids;
     struct nsi_ndis_ifinfo_rw *rw;
     struct nsi_ndis_ifinfo_dynamic *dyn;
@@ -1262,6 +1262,12 @@ static DWORD adapters_addresses_alloc( ULONG family, ULONG flags, IP_ADAPTER_ADD
                                   (void **)&stat, sizeof(*stat), count, 0 );
     if (err) return err;
 
+    if (!*count)
+    {
+        err = ERROR_NO_DATA;
+        goto err;
+    }
+
     needed = *count * (sizeof(*aa) + ((CHARS_IN_GUID + 1) & ~1) + sizeof(stat->descr.String));
     needed += *count * sizeof(rw->alias.String); /* GAA_FLAG_SKIP_FRIENDLY_NAME is ignored */
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10628

[PATCH 0/3] MR10628: nsiproxy.sys: Avoid null pointer dereference on if_nameindex() failure. - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10628