[PATCH v2 1/2] kernelbase: Fix GetSystemFirmwareTable regression
In 71aba33f, GetSystemFirmwareTable was moved from kernel32 to kernelbase. In the process, its error handling was refactored to use NtQuerySystemInformation, and it began to return zero whenever NtQuerySystemInformation failed. This prevents software from being able to make 'preflight' calls to determine how large their buffer should be. This patch simply restores the old behavior. Signed-off-by: John Chadwick <john(a)jchw.io> --- dlls/kernelbase/memory.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 6be399bc97..c93d490e74 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -1161,13 +1161,10 @@ UINT WINAPI GetSystemFirmwareTable( DWORD provider, DWORD id, void *buffer, DWOR info->Action = SystemFirmwareTable_Get; info->TableID = id; - if (set_ntstatus( NtQuerySystemInformation( SystemFirmwareTableInformation, - info, buffer_size, &buffer_size ))) - { - buffer_size -= offsetof( SYSTEM_FIRMWARE_TABLE_INFORMATION, TableBuffer ); - if (buffer_size <= size) memcpy( buffer, info->TableBuffer, buffer_size ); - } - else buffer_size = 0; + set_ntstatus( NtQuerySystemInformation( SystemFirmwareTableInformation, + info, buffer_size, &buffer_size )); + 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; -- 2.23.0
This adds a test to ensure that a typical usage of GetSystemFirmwareTable with a preflight call works correctly. Signed-off-by: John Chadwick <john(a)jchw.io> --- dlls/kernelbase/tests/Makefile.in | 1 + dlls/kernelbase/tests/memory.c | 62 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 dlls/kernelbase/tests/memory.c diff --git a/dlls/kernelbase/tests/Makefile.in b/dlls/kernelbase/tests/Makefile.in index 22e4a17a58..f269147733 100644 --- a/dlls/kernelbase/tests/Makefile.in +++ b/dlls/kernelbase/tests/Makefile.in @@ -1,5 +1,6 @@ TESTDLL = kernelbase.dll C_SRCS = \ + memory.c \ path.c \ sync.c diff --git a/dlls/kernelbase/tests/memory.c b/dlls/kernelbase/tests/memory.c new file mode 100644 index 0000000000..f68077678a --- /dev/null +++ b/dlls/kernelbase/tests/memory.c @@ -0,0 +1,62 @@ +/* + * Memory tests for kernelbase.dll + * + * Copyright 2019 John Chadwick + * + * 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 <windef.h> +#include <winbase.h> +#include <stdlib.h> +#include <winerror.h> + +#include "wine/test.h" + +#define RSMB 0x52534D42 + +static UINT (WINAPI *pGetSystemFirmwareTable)(DWORD, DWORD, void *, DWORD); + +static void test_GetSystemFirmwareTable(void) +{ + DWORD size, bytes_returned; + void *data; + + if (!pGetSystemFirmwareTable) + { + win_skip("GetSystemFirmwareTable not available\n"); + return; + } + + size = pGetSystemFirmwareTable(RSMB, 0, NULL, 0); + ok(size > 0, "got %d", size); + + data = HeapAlloc(GetProcessHeap(), 0, size); + ok(data != NULL, "alloc failed"); + + bytes_returned = pGetSystemFirmwareTable(RSMB, 0, data, size); + ok(bytes_returned == size, "expect %d, got %d\n", size, bytes_returned); +} + +START_TEST(memory) +{ + HMODULE hmod; + + hmod = LoadLibraryA("kernelbase.dll"); + pGetSystemFirmwareTable = (void *)GetProcAddress(hmod, "GetSystemFirmwareTable"); + + test_GetSystemFirmwareTable(); +} -- 2.23.0
On 11/20/19 3:07 PM, John Chadwick wrote:
This adds a test to ensure that a typical usage of GetSystemFirmwareTable with a preflight call works correctly.
Signed-off-by: John Chadwick <john(a)jchw.io> --- dlls/kernelbase/tests/Makefile.in | 1 + dlls/kernelbase/tests/memory.c | 62 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 dlls/kernelbase/tests/memory.c
diff --git a/dlls/kernelbase/tests/Makefile.in b/dlls/kernelbase/tests/Makefile.in index 22e4a17a58..f269147733 100644 --- a/dlls/kernelbase/tests/Makefile.in +++ b/dlls/kernelbase/tests/Makefile.in @@ -1,5 +1,6 @@ TESTDLL = kernelbase.dll
C_SRCS = \ + memory.c \ path.c \ sync.c diff --git a/dlls/kernelbase/tests/memory.c b/dlls/kernelbase/tests/memory.c new file mode 100644 index 0000000000..f68077678a --- /dev/null +++ b/dlls/kernelbase/tests/memory.c @@ -0,0 +1,62 @@ +/* + * Memory tests for kernelbase.dll + * + * Copyright 2019 John Chadwick + * + * 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 <windef.h> +#include <winbase.h> +#include <stdlib.h> +#include <winerror.h> + +#include "wine/test.h" + +#define RSMB 0x52534D42 + +static UINT (WINAPI *pGetSystemFirmwareTable)(DWORD, DWORD, void *, DWORD); + +static void test_GetSystemFirmwareTable(void) +{ + DWORD size, bytes_returned; + void *data; + + if (!pGetSystemFirmwareTable) + { + win_skip("GetSystemFirmwareTable not available\n"); + return; + } + + size = pGetSystemFirmwareTable(RSMB, 0, NULL, 0); + ok(size > 0, "got %d", size); + + data = HeapAlloc(GetProcessHeap(), 0, size); + ok(data != NULL, "alloc failed"); Hi John,
kernel32/tests/version.c already has test_GetSystemFirmwareTable(). So I think you should add the tests there. And it would be better to add the tests first to demonstrate the breakage (adding a todo_wine) and a fix after that to remove todo_wine. Also data is leaked. Thanks, Zhiyi
+ + bytes_returned = pGetSystemFirmwareTable(RSMB, 0, data, size); + ok(bytes_returned == size, "expect %d, got %d\n", size, bytes_returned); +} + +START_TEST(memory) +{ + HMODULE hmod; + + hmod = LoadLibraryA("kernelbase.dll"); + pGetSystemFirmwareTable = (void *)GetProcAddress(hmod, "GetSystemFirmwareTable"); + + test_GetSystemFirmwareTable(); +}
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=60503 Your paranoid android. === w864 (task log) === Task errors: The previous 1 run(s) terminated abnormally
participants (3)
-
John Chadwick -
Marvin -
Zhiyi Zhang