[PATCH v2 0/2] MR1978: nsi: Fix two NsiAllocateAndGetTable() bugs

[Francois Gouget (＠fgouget)]

--
  v2: nsi: Add some margin to the tables size in NsiAllocateAndGetTable().
      nsi: Return an error if NsiAllocateAndGetTable() fails to allocate the tables.

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

[PATCH v2 1/2] nsi: Return an error if NsiAllocateAndGetTable() fails to allocate the tables.

[Francois Gouget]

From: Francois Gouget

err must be set if the last attempt to allocate the tables fails.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54328
---
 dlls/nsi/nsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/nsi/nsi.c b/dlls/nsi/nsi.c
index 345dce6dfe6..6d8ae40a5c7 100644
--- a/dlls/nsi/nsi.c
+++ b/dlls/nsi/nsi.c
@@ -70,6 +70,7 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR
         memset( data, 0, sizeof(data) );
         err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, NULL, 0, NULL, 0, NULL, 0, NULL, 0, &num );
         if (err) return err;
+        err = ERROR_OUTOFMEMORY; /* fail if this is the last attempt */
     }

if (!err)

[PATCH v2 2/2] nsi: Add some margin to the tables size in NsiAllocateAndGetTable().

[Francois Gouget]

From: Francois Gouget

The table size typically keeps increasing such that in some cases the
size we got is already too small by the time we try to get the table
content in the next iteration.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54328
---
This issue regularly causes NsiAllocateAndGetTable() to fail on my
Linux box, maybe because there is always some network traffic.
It would go like this: num=64 -> too small, num=264 -> too small,
num=266 -> too small, num=267 -> too small, eventualy wasting all 5
attempts (and causing a crash when NsiAllocateAndGetTable() returned
success despite it all).
---
 dlls/nsi/nsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/nsi/nsi.c b/dlls/nsi/nsi.c
index 6d8ae40a5c7..3f324ef555b 100644
--- a/dlls/nsi/nsi.c
+++ b/dlls/nsi/nsi.c
@@ -71,6 +71,7 @@ DWORD WINAPI NsiAllocateAndGetTable( DWORD unk, const NPI_MODULEID *module, DWOR
         err = NsiEnumerateObjectsAllParameters( unk, 0, module, table, NULL, 0, NULL, 0, NULL, 0, NULL, 0, &num );
         if (err) return err;
         err = ERROR_OUTOFMEMORY; /* fail if this is the last attempt */
+        num += num >> 4; /* the tables may grow before the next iteration; get ahead */
     }

if (!err)

[Francois Gouget (＠fgouget)]

v2: only set err in the first commit; reword the comment in the second commit

[Francois Gouget (＠fgouget)]

One thing to note is that when I only apply the first commit, NsiAllocateAndGetTable() sometimes fails to allocate the tables. This results in both the NsiAllocateAndGetTable() and GetExtendedTcpTable() calls to fail in test_tcp_tables() which is quickly followed by a crash:

nsi.c:872: Test failed: AF_INET: 8: got 14 /* ERROR_OUTOFMEMORY */
nsi.c:881: Test failed: AF_INET: 8: got 122 /* ERROR_INSUFFICIENT_BUFFER */
wine: Unhandled page fault on read access to 0116B1F4 at address 00401336 (thread 0024), starting debugger...
[...]
Backtrace:
=>0 0x00401336 test_tcp_tables+0x317(family=<internal error>, table_type=<internal error>) [Z:\home\fgouget\wine\wine-gitlab\dlls\nsi\tests\nsi
.c:901] in nsi_test (0x0064fdb8)

That makes sense since dyn_tbl was left uninitialized by NsiAllocateAndGetTable().

So one could say that the test is buggy and should take this case into account.
Or one can consider that it is up to NsiAllocateAndGetTable() to not fail to allocate the table over trivial issues, which seems to be the whole point of the 5 attempts loop. So that's why I also included the second patch which completely eliminates these failures on my box.

[PATCH v2 0/2] MR1978: nsi: Fix two NsiAllocateAndGetTable() bugs - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.