[PATCH v2 0/1] MR425: ntdll: Do not use find_subheap() to find the subheap corresponding to an uncommitted region.

[Zebediah Figura (＠zfigura)]

The commit end of a subheap may be equal to the beginning of another subheap, in
which case find_subheap() will return that one, and we may effectively skip
backwards in the subheap list.

---

There may be a more architecturally palatable way to solve this problem, but
hopefully this is enough to at least demonstrate it.

--
  v2: ntdll: Do not use find_subheap() to find the subheap corresponding to an uncommitted region.

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

[PATCH v2 1/1] ntdll: Do not use find_subheap() to find the subheap corresponding to an uncommitted region.

[Zebediah Figura]

From: Zebediah Figura

The commit end of a subheap may be equal to the beginning of another subheap, in
which case find_subheap() will return that one, and we may effectively skip
backwards in the subheap list.

This fixes a hang on starting or loading a game with Bloodrayne: Terminal Cut.

---

There may be a more architecturally palatable way to solve this problem, but
hopefully this is enough to at least demonstrate it.
---
 dlls/ntdll/heap.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c
index 40c317417ce..4cc494c358b 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -623,6 +623,20 @@ static SUBHEAP *find_subheap( const struct heap *heap, const struct block *block
 }

+static SUBHEAP *find_subheap_by_commit_end( const struct heap *heap, const void *commit_end )
+{
+    SUBHEAP *subheap;
+
+    LIST_FOR_EACH_ENTRY( subheap, &heap->subheap_list, SUBHEAP, entry )
+    {
+        if (subheap_commit_end( subheap ) == commit_end)
+            return subheap;
+    }
+
+    return NULL;
+}
+
+
 static inline BOOL subheap_commit( const struct heap *heap, SUBHEAP *subheap, const struct block *block, SIZE_T block_size )
 {
     const char *end = (char *)subheap_base( subheap ) + subheap_size( subheap ), *commit_end;
@@ -1843,13 +1857,18 @@ static NTSTATUS heap_walk_blocks( const struct heap *heap, const SUBHEAP *subhea

static NTSTATUS heap_walk( const struct heap *heap, struct rtl_heap_entry *entry )
 {
-    const ARENA_LARGE *large;
+    const ARENA_LARGE *large = NULL;
     const struct list *next;
     const SUBHEAP *subheap;
     NTSTATUS status;
     char *base;

-    if ((large = find_large_block( heap, entry->lpData )))
+    if (entry->wFlags & RTL_HEAP_ENTRY_UNCOMMITTED)
+    {
+        subheap = find_subheap_by_commit_end( heap, entry->lpData );
+        next = &subheap->entry;
+    }
+    else if ((large = find_large_block( heap, entry->lpData )))
         next = &large->entry;
     else if ((subheap = find_subheap( heap, entry->lpData, TRUE )))
     {

[PATCH v2 1/1] ntdll: Do not use find_subheap() to find the subheap corresponding to an uncommitted region.

[Zebediah Figura]

I didn't include a test with this because I don't think it's possible to 
reliably trigger the condition. It relies on an allocated block 
stretching to the end of a subheap (which I think it's possible to make 
happen reliably, although it'd end up relying on internal heap details 
to some degree) but also on two subsequent subheaps being allocated to 
contiguous VM slices in reverse order.

I'm able to consistently trigger it (on 32-bit) with the attached diff 
to the heap tests, though.

[PATCH v2 0/1] MR425: ntdll: Do not use find_subheap() to find the subheap corresponding to an uncommitted region. - closed

[Zebediah Figura (＠zfigura)]

This merge request was closed by Zebediah Figura.