[PATCH 0/3] MR1397: setupapi: Fixes for buffer size handling in SetupDiGetDeviceInterfaceDetail().
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/setupapi/tests/devinst.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index c4eebdf0ab8..9d6a61c08c8 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1192,7 +1192,7 @@ static void test_device_iface_detail(void) SP_DEVICE_INTERFACE_DETAIL_DATA_A *detail; SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; SP_DEVINFO_DATA device = {sizeof(device)}; - DWORD size = 0, expectedsize; + DWORD size = 0, expected_size; HDEVINFO set; BOOL ret; @@ -1231,37 +1231,57 @@ static void test_device_iface_detail(void) ok(!ret, "Expected failure.\n"); ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); - detail = heap_alloc(size); - expectedsize = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath[strlen(path) + 1]); + expected_size = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_A, DevicePath[strlen(path) + 1]); + todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + detail = heap_alloc(size * 2); detail->cbSize = 0; SetLastError(0xdeadbeef); - ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, size, &size, NULL); + size = 0xdeadbeef; + ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size * 2, &size, NULL); ok(!ret, "Expected failure.\n"); ok(GetLastError() == ERROR_INVALID_USER_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); + ok(size == 0xdeadbeef, "Expected size %lu, got %lu.\n", expected_size, size); detail->cbSize = size; SetLastError(0xdeadbeef); - ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, size, &size, NULL); + size = 0xdeadbeef; + ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size * 2, &size, NULL); ok(!ret, "Expected failure.\n"); ok(GetLastError() == ERROR_INVALID_USER_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); + ok(size == 0xdeadbeef, "Expected size %lu, got %lu.\n", expected_size, size); detail->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_A); SetLastError(0xdeadbeef); - ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, size, &size, NULL); + size = 0xdeadbeef; + ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size, &size, NULL); + todo_wine ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); + if (ret) + ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); + todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + + SetLastError(0xdeadbeef); + size = 0xdeadbeef; + ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size * 2, &size, NULL); ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); + todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + + expected_size = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath[strlen(path) + 1]); + size = 0xdeadbeef; ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, NULL, 0, &size, NULL); ok(!ret, "Expected failure.\n"); ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); - ok(size == expectedsize, "Got unexpected size %ld.\n", size); + ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); memset(&device, 0, sizeof(device)); device.cbSize = sizeof(device); + size = 0xdeadbeef; ret = SetupDiGetDeviceInterfaceDetailW(set, &iface, NULL, 0, &size, &device); ok(!ret, "Expected failure.\n"); ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); + ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); ok(IsEqualGUID(&device.ClassGuid, &guid), "Got unexpected class %s.\n", wine_dbgstr_guid(&device.ClassGuid)); heap_free(detail); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1397
From: Zebediah Figura <zfigura(a)codeweavers.com> Don't include the null terminator twice. --- dlls/setupapi/devinst.c | 2 +- dlls/setupapi/tests/devinst.c | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 901af4650c8..930ed670e84 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -3003,7 +3003,7 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailA(HDEVINFO devinfo, SP_DEVICE_INTERFA if (iface->symlink) bytesNeeded += WideCharToMultiByte(CP_ACP, 0, iface->symlink, -1, - NULL, 0, NULL, NULL); + NULL, 0, NULL, NULL) - 1; if (DeviceInterfaceDetailDataSize >= bytesNeeded) { if (iface->symlink) diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 9d6a61c08c8..2a33da2d72a 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1232,7 +1232,7 @@ static void test_device_iface_detail(void) ok(GetLastError() == ERROR_INSUFFICIENT_BUFFER, "Got unexpected error %#lx.\n", GetLastError()); expected_size = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_A, DevicePath[strlen(path) + 1]); - todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); detail = heap_alloc(size * 2); detail->cbSize = 0; @@ -1255,9 +1255,8 @@ static void test_device_iface_detail(void) SetLastError(0xdeadbeef); size = 0xdeadbeef; ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size, &size, NULL); - todo_wine ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); - if (ret) - ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); + ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); + ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); SetLastError(0xdeadbeef); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1397
From: Zebediah Figura <zfigura(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53387 --- dlls/setupapi/devinst.c | 24 ++++++++++++++---------- dlls/setupapi/tests/devinst.c | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 930ed670e84..896e77d9b38 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -2976,15 +2976,15 @@ BOOL WINAPI SetupDiDestroyDeviceInfoList(HDEVINFO devinfo) */ BOOL WINAPI SetupDiGetDeviceInterfaceDetailA(HDEVINFO devinfo, SP_DEVICE_INTERFACE_DATA *iface_data, SP_DEVICE_INTERFACE_DETAIL_DATA_A *DeviceInterfaceDetailData, - DWORD DeviceInterfaceDetailDataSize, DWORD *RequiredSize, SP_DEVINFO_DATA *device_data) + DWORD DeviceInterfaceDetailDataSize, DWORD *ret_size, SP_DEVINFO_DATA *device_data) { struct device_iface *iface; DWORD bytesNeeded = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_A, DevicePath[1]); BOOL ret = FALSE; - TRACE("devinfo %p, iface_data %p, detail_data %p, size %ld, needed %p, device_data %p.\n", + TRACE("devinfo %p, iface_data %p, detail_data %p, size %ld, ret_size %p, device_data %p.\n", devinfo, iface_data, DeviceInterfaceDetailData, DeviceInterfaceDetailDataSize, - RequiredSize, device_data); + ret_size, device_data); if (!(iface = get_device_iface(devinfo, iface_data))) return FALSE; @@ -3004,6 +3004,10 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailA(HDEVINFO devinfo, SP_DEVICE_INTERFA if (iface->symlink) bytesNeeded += WideCharToMultiByte(CP_ACP, 0, iface->symlink, -1, NULL, 0, NULL, NULL) - 1; + + if (ret_size) + *ret_size = bytesNeeded; + if (DeviceInterfaceDetailDataSize >= bytesNeeded) { if (iface->symlink) @@ -3019,8 +3023,6 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailA(HDEVINFO devinfo, SP_DEVICE_INTERFA } else { - if (RequiredSize) - *RequiredSize = bytesNeeded; SetLastError(ERROR_INSUFFICIENT_BUFFER); } @@ -3035,16 +3037,16 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailA(HDEVINFO devinfo, SP_DEVICE_INTERFA */ BOOL WINAPI SetupDiGetDeviceInterfaceDetailW(HDEVINFO devinfo, SP_DEVICE_INTERFACE_DATA *iface_data, SP_DEVICE_INTERFACE_DETAIL_DATA_W *DeviceInterfaceDetailData, - DWORD DeviceInterfaceDetailDataSize, DWORD *RequiredSize, SP_DEVINFO_DATA *device_data) + DWORD DeviceInterfaceDetailDataSize, DWORD *ret_size, SP_DEVINFO_DATA *device_data) { struct device_iface *iface; DWORD bytesNeeded = offsetof(SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath) + sizeof(WCHAR); /* include NULL terminator */ BOOL ret = FALSE; - TRACE("devinfo %p, iface_data %p, detail_data %p, size %ld, needed %p, device_data %p.\n", + TRACE("devinfo %p, iface_data %p, detail_data %p, size %ld, ret_size %p, device_data %p.\n", devinfo, iface_data, DeviceInterfaceDetailData, DeviceInterfaceDetailDataSize, - RequiredSize, device_data); + ret_size, device_data); if (!(iface = get_device_iface(devinfo, iface_data))) return FALSE; @@ -3064,6 +3066,10 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailW(HDEVINFO devinfo, SP_DEVICE_INTERFA if (iface->symlink) bytesNeeded += sizeof(WCHAR) * lstrlenW(iface->symlink); + + if (ret_size) + *ret_size = bytesNeeded; + if (DeviceInterfaceDetailDataSize >= bytesNeeded) { if (iface->symlink) @@ -3075,8 +3081,6 @@ BOOL WINAPI SetupDiGetDeviceInterfaceDetailW(HDEVINFO devinfo, SP_DEVICE_INTERFA } else { - if (RequiredSize) - *RequiredSize = bytesNeeded; SetLastError(ERROR_INSUFFICIENT_BUFFER); } diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c index 2a33da2d72a..be0831f6882 100644 --- a/dlls/setupapi/tests/devinst.c +++ b/dlls/setupapi/tests/devinst.c @@ -1257,14 +1257,14 @@ static void test_device_iface_detail(void) ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size, &size, NULL); ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); - todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); SetLastError(0xdeadbeef); size = 0xdeadbeef; ret = SetupDiGetDeviceInterfaceDetailA(set, &iface, detail, expected_size * 2, &size, NULL); ok(ret, "Failed to get interface detail, error %#lx.\n", GetLastError()); ok(!strcasecmp(path, detail->DevicePath), "Got unexpected path %s.\n", detail->DevicePath); - todo_wine ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); + ok(size == expected_size, "Expected size %lu, got %lu.\n", expected_size, size); expected_size = FIELD_OFFSET(SP_DEVICE_INTERFACE_DETAIL_DATA_W, DevicePath[strlen(path) + 1]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1397
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126260 Your paranoid android. === debian11 (32 bit report) === httpapi: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x7b020c3f).
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (＠zfigura)