[PATCH 0/2] MR6271: odbc32 fix creating descriptor
Passing a null into create_descriptor crashes on .... init_object( &ret->hdr, SQL_HANDLE_DESC, &stmt->hdr ); -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/odbc32/proxyodbc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/odbc32/proxyodbc.c b/dlls/odbc32/proxyodbc.c index 299f1f8e08b..8ab64373288 100644 --- a/dlls/odbc32/proxyodbc.c +++ b/dlls/odbc32/proxyodbc.c @@ -395,7 +395,11 @@ static void init_object( struct object *obj, UINT32 type, struct object *parent obj->parent = parent; list_init( &obj->entry ); list_init( &obj->children ); - if (parent) list_add_tail( &parent->children, &obj->entry ); + if (parent) + { + list_add_tail( &parent->children, &obj->entry ); + obj->win32_funcs = parent->win32_funcs; + } InitializeCriticalSectionEx( &obj->cs, 0, RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO ); obj->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": object.cs"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6271
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/odbc32/proxyodbc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/odbc32/proxyodbc.c b/dlls/odbc32/proxyodbc.c index 8ab64373288..f07a6510d7f 100644 --- a/dlls/odbc32/proxyodbc.c +++ b/dlls/odbc32/proxyodbc.c @@ -544,7 +544,7 @@ static SQLRETURN alloc_descriptors( struct statement *stmt ) unsigned int i; for (i = 0; i < ARRAY_SIZE(stmt->desc); i++) { - if (!(stmt->desc[i] = create_descriptor( NULL ))) + if (!(stmt->desc[i] = create_descriptor( stmt ))) { free_descriptors( stmt ); return SQL_ERROR; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6271
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147723 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/odbc32/proxyodbc.c:544 Task: Patch failed to apply === debian11b (build log) === error: patch failed: dlls/odbc32/proxyodbc.c:544 Task: Patch failed to apply
Hans Leidekker (@hans) commented about dlls/odbc32/proxyodbc.c:
unsigned int i; for (i = 0; i < ARRAY_SIZE(stmt->desc); i++) { - if (!(stmt->desc[i] = create_descriptor( NULL ))) + if (!(stmt->desc[i] = create_descriptor( stmt )))
This is on purpose, these descriptors are not regular child objects. Can you describe what fails? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271#note_78626
On Wed Aug 14 07:01:39 2024 +0000, Hans Leidekker wrote:
This is on purpose, these descriptors are not regular child objects. Can you describe what fails? First, it's using NULL as an offset to a pointer structure
``` descriptor *create_descriptor( struct statement *stmt ) ... init_object( &ret->hdr, SQL_HANDLE_DESC, &stmt->hdr ); Second a crash in set_desc_rec_win32, assumes that win32_funcs is valid. SQLSetDescRec ... struct descriptor *desc = (struct descriptor *)lock_object( DescriptorHandle, SQL_HANDLE_DESC ); -> set_desc_rec_win32(desc) -> -> desc->hdr.win32_funcs->SQLSetDescRec ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271#note_78627
On Wed Aug 14 07:16:37 2024 +0000, Alistair Leslie-Hughes wrote:
First, it's using NULL as an offset to a pointer structure ``` descriptor *create_descriptor( struct statement *stmt ) ... init_object( &ret->hdr, SQL_HANDLE_DESC, &stmt->hdr ); Second a crash in set_desc_rec_win32, assumes that win32_funcs is valid. SQLSetDescRec ... struct descriptor *desc = (struct descriptor *)lock_object( DescriptorHandle, SQL_HANDLE_DESC ); -> set_desc_rec_win32(desc) -> -> desc->hdr.win32_funcs->SQLSetDescRec ``` hdr is the first field so &stmt->hdr is NULL when stmt is NULL. win32_funcs should be set in alloc_handle(). If that's not the case then there must be a bug somewhere else. Can you show you are calling these funtions?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271#note_78628
On Wed Aug 14 07:29:28 2024 +0000, Hans Leidekker wrote:
hdr is the first field so &stmt->hdr is NULL when stmt is NULL. win32_funcs should be set in alloc_handle(). If that's not the case then there must be a bug somewhere else. Can you show you are calling these funtions?
Backtrace of the crash =>0 0x7537ee50 set_desc_field_win32_w(desc=<internal error>, len=<internal error>, value=<internal error>, id=<internal error>, record=<internal error>) [/home/alesliehughes/wine-staging/wine32/../wine/dlls/odbc32/proxyodbc.c:7600] in odbc32 (0x077ee540) 1 0x7537ee50 SQLSetDescFieldW+0x80(DescriptorHandle=<couldn't compute location>, RecNumber=<couldn't compute location>, FieldIdentifier=<couldn't compute location>, Value=<couldn't compute location>, BufferLength=<couldn't compute location>) [/home/alesliehughes/wine-staging/wine32/../wine/dlls/odbc32/proxyodbc.c:7629] in odbc32 (0x077ee540) 2 0x1f6b50c7 in msdasql (+0x50c7) (0x077ee578) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271#note_78629
On Wed Aug 14 07:38:57 2024 +0000, Alistair Leslie-Hughes wrote:
Backtrace of the crash =>0 0x7537ee50 set_desc_field_win32_w(desc=<internal error>, len=<internal error>, value=<internal error>, id=<internal error>, record=<internal error>) [/home/alesliehughes/wine-staging/wine32/../wine/dlls/odbc32/proxyodbc.c:7600] in odbc32 (0x077ee540) 1 0x7537ee50 SQLSetDescFieldW+0x80(DescriptorHandle=<couldn't compute location>, RecNumber=<couldn't compute location>, FieldIdentifier=<couldn't compute location>, Value=<couldn't compute location>, BufferLength=<couldn't compute location>) [/home/alesliehughes/wine-staging/wine32/../wine/dlls/odbc32/proxyodbc.c:7629] in odbc32 (0x077ee540) 2 0x1f6b50c7 in msdasql (+0x50c7) (0x077ee578) Thanks. I have added a patch in MR !6274 that should fix this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271#note_78632
This merge request was closed by Alistair Leslie-Hughes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6271
participants (4)
-
Alistair Leslie-Hughes -
Alistair Leslie-Hughes (@alesliehughes) -
Hans Leidekker (@hans) -
Marvin