[PATCH v2 0/2] MR6929: kernelbase: Properly return 0 from EnumSystemFirmwareTable on error.
6d1df55bf3b6051ad61e9c7cb7d63f13c1b9771f changed `NtQuerySystemInformation(SystemFirmwareTableInformation, ...)` to return 0 on error, which left `get_firmware_table` returning -16 This would be interpreted as a very large number by applications, breaking them. -- v2: kernelbase: Add test for EnumSystemFirmwareTables on missing provider. https://gitlab.winehq.org/wine/wine/-/merge_requests/6929
From: Evan Tang <etang(a)codeweavers.com> --- dlls/kernelbase/memory.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index fde655eaaed..0be178f6ab7 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -1790,6 +1790,7 @@ static UINT get_firmware_table( DWORD provider, SYSTEM_FIRMWARE_TABLE_ACTION act { SYSTEM_FIRMWARE_TABLE_INFORMATION *info; ULONG buffer_size = offsetof( SYSTEM_FIRMWARE_TABLE_INFORMATION, TableBuffer ) + size; + NTSTATUS status; if (!(info = RtlAllocateHeap( GetProcessHeap(), 0, buffer_size ))) { @@ -1801,13 +1802,13 @@ static UINT get_firmware_table( DWORD provider, SYSTEM_FIRMWARE_TABLE_ACTION act info->Action = action; info->TableID = id; - set_ntstatus( NtQuerySystemInformation( SystemFirmwareTableInformation, - info, buffer_size, &buffer_size )); + status = NtQuerySystemInformation( SystemFirmwareTableInformation, info, buffer_size, &buffer_size ); + set_ntstatus(status); buffer_size -= offsetof( SYSTEM_FIRMWARE_TABLE_INFORMATION, TableBuffer ); if (buffer_size <= size) memcpy( buffer, info->TableBuffer, buffer_size ); HeapFree( GetProcessHeap(), 0, info ); - return buffer_size; + return NT_SUCCESS(status) || status == STATUS_BUFFER_TOO_SMALL ? buffer_size : 0; } /*********************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6929
From: Evan Tang <etang(a)codeweavers.com> --- dlls/kernelbase/tests/Makefile.in | 1 + dlls/kernelbase/tests/memory.c | 55 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 dlls/kernelbase/tests/memory.c diff --git a/dlls/kernelbase/tests/Makefile.in b/dlls/kernelbase/tests/Makefile.in index 654cb9d5aed..16d3afe0dc8 100644 --- a/dlls/kernelbase/tests/Makefile.in +++ b/dlls/kernelbase/tests/Makefile.in @@ -2,6 +2,7 @@ TESTDLL = kernelbase.dll SOURCES = \ file.c \ + memory.c \ path.c \ process.c \ rsrc.rc \ diff --git a/dlls/kernelbase/tests/memory.c b/dlls/kernelbase/tests/memory.c new file mode 100644 index 00000000000..b7841d57fee --- /dev/null +++ b/dlls/kernelbase/tests/memory.c @@ -0,0 +1,55 @@ +/* + * Copyright (C) 2023 Paul Gofman for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <stdarg.h> +#include <stdlib.h> + +#include <ntstatus.h> +#define WIN32_NO_STATUS +#include <windef.h> +#include <winbase.h> +#include <winerror.h> + +#include "wine/test.h" + +static UINT WINAPI (WINAPI *pEnumSystemFirmwareTables)(DWORD provider, void *buffer, DWORD size); + +static void test_enum_system_firmware_tables(void) +{ + UINT res; + DWORD err; + + /* Applications may use e.g. 'ACPI', which we currently don't support. + * We should at the very least return valid error codes for it. + * Test with a definitely-invalid provider so it also fails on real Windows. */ + SetLastError(0xdeadbeef); + res = pEnumSystemFirmwareTables(~0u, NULL, 0); + err = GetLastError(); + ok(res == 0, "Unexpected firmware table count for invalid provider: %d\n", res); + ok(err == ERROR_INVALID_FUNCTION, "Unexpected error for invalid provider: %ld\n", err); +} + +START_TEST(memory) +{ + HMODULE hmod; + + hmod = LoadLibraryA("kernelbase.dll"); + pEnumSystemFirmwareTables = (void *)GetProcAddress(hmod, "EnumSystemFirmwareTables"); + + test_enum_system_firmware_tables(); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6929
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=150091 Your paranoid android. === w7u_2qxl (32 bit report) === kernelbase: 0634:memory: unhandled exception c0000005 at 00000000 === w7u_adm (32 bit report) === kernelbase: 092c:memory: unhandled exception c0000005 at 00000000 === w7u_el (32 bit report) === kernelbase: 0a74:memory: unhandled exception c0000005 at 00000000 === w7pro64 (64 bit report) === kernelbase: 0720:memory: unhandled exception c0000005 at 0000000000000000 === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000006C00E6, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 win.c:4070: Test failed: Expected active window 00000000077F0154, got 0000000000000000. win.c:4071: Test failed: Expected focus window 00000000077F0154, got 0000000000000000.
participants (3)
-
Evan Tang -
Evan Tang (＠etang-cw)
-
Marvin