[PATCH v10 0/4] MR10278: d3dxof: Pad with magic value before object data in parse_object().
Space Empires V interprets data returned from (IDirectXFileData_GetData()-4) as refcount, reads and writes to it and makes allocation decisions based on it. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49722 -- v10: Apply 1 suggestion(s) to 1 file(s) https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
From: Joerg Rueppel <joru.ossdev@gmail.com> Space Empires V interprets data returned from (IDirectXFileData_GetData()-4) as refcount, reads and writes to it and makes allocation decisions based on it. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49722 --- dlls/d3dxof/parsing.c | 12 +++- dlls/d3dxof/tests/d3dxof.c | 116 +++++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/dlls/d3dxof/parsing.c b/dlls/d3dxof/parsing.c index 149e2371168..92df0427367 100644 --- a/dlls/d3dxof/parsing.c +++ b/dlls/d3dxof/parsing.c @@ -1362,10 +1362,20 @@ BOOL parse_object(parse_buffer * buf) { ULONG i; + buf->pxo->root = buf->pxo_tab; + + /* Insert padding for case where the returned object gets reinterpreted as + * Delphi dynamic array, which will write to ptr-4 for refcounting and + * will use the preexisting value for allocation decisions. Must be >= 2. + */ + if (!check_buffer(buf, 4)) + return FALSE; + *(DWORD *)(buf->pdata + buf->cur_pos_data) = 2; + buf->cur_pos_data += 4; + buf->pxo->pos_data = buf->cur_pos_data; buf->pxo->ptarget = NULL; buf->pxo->binary = FALSE; - buf->pxo->root = buf->pxo_tab; if (get_TOKEN(buf) != TOKEN_NAME) return FALSE; diff --git a/dlls/d3dxof/tests/d3dxof.c b/dlls/d3dxof/tests/d3dxof.c index b7b1b1cc502..e085c1f9bfa 100644 --- a/dlls/d3dxof/tests/d3dxof.c +++ b/dlls/d3dxof/tests/d3dxof.c @@ -366,6 +366,8 @@ static char object_complex[] = "3;;;, 0;;;, 1;;;, 2;;;,\n" "3;;;, 1;;;, 2;;;, 3;;;,\n" "3;;;, 3;;;, 1;;;, 2;;;,\n" +/* child object for heap alloc test */ +"Vector{0.0;;;, 0.0;;;, 0.0;;;}\n" "}\n"; static char template_using_index_color_lower[] = @@ -1005,6 +1007,119 @@ static void test_complex_object(void) IDirectXFile_Release(dxfile); } +static void test_getdata_memory_layout(void) +{ + IDirectXFile *dxfile = NULL; + IDirectXFileEnumObject *enum_object, *enum_object2; + IDirectXFileData *root_data, *root_data2, *child_data, *child_data2; + IDirectXFileObject *child_obj, *child_obj2; + DXFILELOADMEMORY load_info; + HRESULT hr; + size_t distance; + DWORD root_size, child_size; + void *root_ptr, *child_ptr; + void *root_ptr2, *child_ptr2; + void *pre_data; + ULONG ref; + + if (!pDirectXFileCreate) + { + win_skip("DirectXFileCreate is not available\n"); + return; + } + + hr = pDirectXFileCreate(&dxfile); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + + hr = IDirectXFile_RegisterTemplates(dxfile, templates_complex_object, sizeof(templates_complex_object) - 1); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + + load_info.lpMemory = object_complex; + load_info.dSize = sizeof(object_complex) - 1; + hr = IDirectXFile_CreateEnumObject(dxfile, &load_info, DXFILELOAD_FROMMEMORY, &enum_object); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + + hr = IDirectXFileEnumObject_GetNextDataObject(enum_object, &root_data); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + hr = IDirectXFileData_GetData(root_data, NULL, &root_size, &root_ptr); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(root_size == 104, "Unexpected root size %lu.\n", root_size); + + hr = IDirectXFileData_GetNextObject(root_data, &child_obj); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + + hr = IDirectXFileObject_QueryInterface(child_obj, &IID_IDirectXFileData, (void** )&child_data); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + hr = IDirectXFileData_GetData(child_data, NULL, &child_size, &child_ptr); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(child_size == 12, "Unexpected child size %lu.\n", child_size); + + distance = (root_ptr > child_ptr) ? (BYTE*) root_ptr - (BYTE*) child_ptr : (BYTE*) child_ptr - (BYTE*) root_ptr; + todo_wine ok(distance > root_size + child_size, "Unexpected distance %Iu.\n", distance); + + pre_data = *((void **)root_ptr - 1); + trace("root_ptr %p, pre_data %p\n", root_ptr, pre_data); + ok(*(DWORD *)((BYTE *)root_ptr - 4) >= 2, "Unexpected value %#lx at root_ptr-4.\n", *(DWORD *)((BYTE *)root_ptr - 4)); + pre_data = *((void **)child_ptr - 1); + trace("child_ptr %p, pre_data %p\n", child_ptr, pre_data); + + hr = IDirectXFileData_GetData(root_data, NULL, &root_size, &root_ptr2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + hr = IDirectXFileData_GetData(child_data, NULL, &child_size, &child_ptr2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(root_ptr == root_ptr2, "Unexpected root_ptr %p, root_ptr2 %p\n", root_ptr, root_ptr2); + ok(child_ptr == child_ptr2, "Unexpected child_ptr %p, child_ptr2 %p\n", child_ptr, child_ptr2); + + hr = IDirectXFile_CreateEnumObject(dxfile, &load_info, DXFILELOAD_FROMMEMORY, &enum_object2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(enum_object != enum_object2, "Unexpected enum_object %p, enum_object2 %p\n", enum_object, enum_object2); + + hr = IDirectXFileEnumObject_GetNextDataObject(enum_object2, &root_data2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(root_data != root_data2, "Unexpected root_data %p, root_data2 %p\n", root_data, root_data2); + hr = IDirectXFileData_GetData(root_data2, NULL, &root_size, &root_ptr2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(root_ptr != root_ptr2, "Unexpected root_ptr %p, root_ptr2 %p\n", root_ptr, root_ptr2); + ok(!memcmp(root_ptr, root_ptr2, root_size), "root node contents differ\n"); + + hr = IDirectXFileData_GetNextObject(root_data2, &child_obj2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(child_obj != child_obj2, "child_obj %p, child_obj2 %p\n", child_obj, child_obj2); + hr = IDirectXFileObject_QueryInterface(child_obj2, &IID_IDirectXFileData, (void** )&child_data2); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(child_data != child_data2, "child_data %p, child_data2 %p\n", child_data, child_data2); + hr = IDirectXFileData_GetData(child_data2, NULL, &child_size, &child_ptr2); + ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); + ok(!memcmp(child_ptr, child_ptr2, child_size), "child node contents differ\n"); + + *(DWORD *)child_ptr = 0x12345678; + ok(!memcmp(root_ptr, root_ptr2, root_size), "root node contents differ\n"); + ok(memcmp(child_ptr, child_ptr2, child_size), "child node contents match\n"); + + todo_wine { + ref = IDirectXFileData_Release(child_data2); + ok(ref == 3, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFileData_Release(child_obj2); + ok(ref == 2, "Unexpected refcount %lu.\n", ref); + } + ref = IDirectXFileData_Release(root_data2); + ok(ref == 1, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFileEnumObject_Release(enum_object2); + ok(!ref, "Unexpected refcount %lu.\n", ref); + + ref = IDirectXFileEnumObject_Release(enum_object); + ok(!ref, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFileData_Release(root_data); + ok(!ref, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFileData_Release(child_data); + ok(ref == 1, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFileObject_Release(child_obj); + ok(!ref, "Unexpected refcount %lu.\n", ref); + ref = IDirectXFile_Release(dxfile); + ok(!ref, "Unexpected refcount %lu.\n", ref); +} + static void test_standard_templates(void) { IDirectXFile *dxfile = NULL; @@ -1061,6 +1176,7 @@ START_TEST(d3dxof) test_syntax(); test_syntax_semicolon_comma(); test_complex_object(); + test_getdata_memory_layout(); test_standard_templates(); test_type_index_color(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
From: Joerg Rueppel <sharky-x@gmx.net> Co-authored-by: Matteo Bruni <matteo.mystral@gmail.com> --- dlls/d3dxof/parsing.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/d3dxof/parsing.c b/dlls/d3dxof/parsing.c index 92df0427367..9336194af84 100644 --- a/dlls/d3dxof/parsing.c +++ b/dlls/d3dxof/parsing.c @@ -1364,10 +1364,11 @@ BOOL parse_object(parse_buffer * buf) buf->pxo->root = buf->pxo_tab; - /* Insert padding for case where the returned object gets reinterpreted as - * Delphi dynamic array, which will write to ptr-4 for refcounting and - * will use the preexisting value for allocation decisions. Must be >= 2. - */ + /* Applications have been known to depend on the presence of padding before + * the object data. + * Specifically, Space Empires V reinterprets the returned object as a + * Delphi dynamic array and updates the value at (DWORD *)ptr - 1 for + * refcounting and allocation decisions. Must be >= 2. */ if (!check_buffer(buf, 4)) return FALSE; *(DWORD *)(buf->pdata + buf->cur_pos_data) = 2; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
From: Joerg Rueppel <sharky-x@gmx.net> Co-authored-by: Matteo Bruni <matteo.mystral@gmail.com> --- dlls/d3dxof/parsing.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/d3dxof/parsing.c b/dlls/d3dxof/parsing.c index 9336194af84..12123f41429 100644 --- a/dlls/d3dxof/parsing.c +++ b/dlls/d3dxof/parsing.c @@ -1369,7 +1369,7 @@ BOOL parse_object(parse_buffer * buf) * Specifically, Space Empires V reinterprets the returned object as a * Delphi dynamic array and updates the value at (DWORD *)ptr - 1 for * refcounting and allocation decisions. Must be >= 2. */ - if (!check_buffer(buf, 4)) + if (!check_buffer(buf, sizeof(DWORD))) return FALSE; *(DWORD *)(buf->pdata + buf->cur_pos_data) = 2; buf->cur_pos_data += 4; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
From: Joerg Rueppel <sharky-x@gmx.net> Co-authored-by: Matteo Bruni <matteo.mystral@gmail.com> --- dlls/d3dxof/parsing.c | 4 ++-- dlls/d3dxof/tests/d3dxof.c | 4 +--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dlls/d3dxof/parsing.c b/dlls/d3dxof/parsing.c index 12123f41429..2e89b42a8d9 100644 --- a/dlls/d3dxof/parsing.c +++ b/dlls/d3dxof/parsing.c @@ -1371,8 +1371,8 @@ BOOL parse_object(parse_buffer * buf) * refcounting and allocation decisions. Must be >= 2. */ if (!check_buffer(buf, sizeof(DWORD))) return FALSE; - *(DWORD *)(buf->pdata + buf->cur_pos_data) = 2; - buf->cur_pos_data += 4; + *(DWORD *)(buf->pdata + buf->cur_pos_data) = 0x55555555; + buf->cur_pos_data += sizeof(DWORD); buf->pxo->pos_data = buf->cur_pos_data; buf->pxo->ptarget = NULL; diff --git a/dlls/d3dxof/tests/d3dxof.c b/dlls/d3dxof/tests/d3dxof.c index e085c1f9bfa..990c1e3bd80 100644 --- a/dlls/d3dxof/tests/d3dxof.c +++ b/dlls/d3dxof/tests/d3dxof.c @@ -366,7 +366,7 @@ static char object_complex[] = "3;;;, 0;;;, 1;;;, 2;;;,\n" "3;;;, 1;;;, 2;;;, 3;;;,\n" "3;;;, 3;;;, 1;;;, 2;;;,\n" -/* child object for heap alloc test */ +/* child object for memory layout test */ "Vector{0.0;;;, 0.0;;;, 0.0;;;}\n" "}\n"; @@ -1059,10 +1059,8 @@ static void test_getdata_memory_layout(void) todo_wine ok(distance > root_size + child_size, "Unexpected distance %Iu.\n", distance); pre_data = *((void **)root_ptr - 1); - trace("root_ptr %p, pre_data %p\n", root_ptr, pre_data); ok(*(DWORD *)((BYTE *)root_ptr - 4) >= 2, "Unexpected value %#lx at root_ptr-4.\n", *(DWORD *)((BYTE *)root_ptr - 4)); pre_data = *((void **)child_ptr - 1); - trace("child_ptr %p, pre_data %p\n", child_ptr, pre_data); hr = IDirectXFileData_GetData(root_data, NULL, &root_size, &root_ptr2); ok(hr == DXFILE_OK, "Unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
Thanks for the update. It looks like accepting the suggestions created multiple separate commits, which is not what we want. I'm going to open a new MR with this all squashed into a single patch, rebased on top of current git. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10278#note_141712
This merge request was closed by Matteo Bruni. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10278
!11029 was just accepted, so your patch is in Wine 11.10 :slight_smile: -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10278#note_141760
On Sun May 31 08:45:49 2026 +0000, Matteo Bruni wrote:
!11029 was just accepted, so your patch is in Wine 11.10 :slight_smile: Thanks! The fixed bug is not mentioned in the release notes though, can it still be updated?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10278#note_141780
On Sun May 31 08:45:49 2026 +0000, Joerg Rueppel wrote:
Thanks! The fixed bug is not mentioned in the release notes though, can it still be updated? The bug has been marked RESOLVED, it will appear in the release notes for Wine 11.11.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10278#note_141788
participants (3)
-
Joerg Rueppel -
Joerg Rueppel (＠jrueppel)
-
Matteo Bruni (＠Mystral)