[PATCH v4 0/4] MR6423: kernel32: Implement GetFirmwareType, GetFirmwareEnvironmentVariable*
Kernel32 Implement: - `GetFirmwareType` - `GetFirmwareEnvironmentVariableExA` - `GetFirmwareEnvironmentVariableExW` - `GetFirmwareEnvironmentVariableA` - `GetFirmwareEnvironmentVariableW`. The code is needed to check SecureBoot status, security systems and for example to check the licenses of software that can use efi variables. Example: ```C #include <stdint.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <windows.h> #include <winnt.h> /* x86_64-w64-mingw32-gcc wuefi.c -o wuefi.exe -Wl,--subsystem,console */ int main(int argc, char **argv) { BOOLEAN secureboot = 0; char asus_license[2048]; FIRMWARE_TYPE type; if(GetFirmwareType(&type)) printf("Firmware Type: %s\n", type == FirmwareTypeUefi ? "UEFI" : "BIOS"); if(GetFirmwareEnvironmentVariableA("SecureBoot", "{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &secureboot, sizeof(BOOLEAN))) printf("SecureBoot: %s\n", secureboot ? "enabled" : "disabled"); if(GetFirmwareEnvironmentVariableW(L"SecureBoot", L"{8be4df61-93ca-11d2-aa0d-00e098032b8c}", &secureboot, sizeof(BOOLEAN))) printf("SecureBoot: %s\n", secureboot ? "enabled" : "disabled"); if(GetFirmwareEnvironmentVariableA("AsusOnboardToolLicense", "02076249-a52b-420e-bd53-aed044349379", asus_license, sizeof(asus_license))) printf("Asus License: \n%s\n\n", asus_license); return 0; } ``` ``` Firmware Type: UEFI SecureBoot: enabled SecureBoot: enabled Asus License: bigtext== [LicenseID]:FED7A1 [Model]:Dummy-BaseBoard-Id [Tools]:WAsusDmiS,LogoFlashS,ASUSFwConfigS,AsusEventLogS,AsusNvLockS [Customer]:DummyLic [Expire]:2022-11-24 ``` -- v4: kernel32: Implement GetFirmwareEnvironmentVariableExA, GetFirmwareEnvironmentVariableExW, GetFirmwareEnvironmentVariableA, GetFirmwareEnvironmentVariableW. kernel32: Implement GetFirmwareType. ntdll: Implement NtQuerySystemInformation SystemBootEnvironmentInformation. ntdll: Implement NtQuerySystemEnvironmentValueEx. https://gitlab.winehq.org/wine/wine/-/merge_requests/6423
From: Grigory Vasilyev <h0tc0d3(a)gmail.com> --- dlls/ntdll/unix/system.c | 59 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index f47925018b8..0d3a4f403ef 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -30,6 +30,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <sys/stat.h> #include <sys/types.h> #include <unistd.h> #include <errno.h> @@ -3682,9 +3683,67 @@ NTSTATUS WINAPI NtQuerySystemEnvironmentValue( UNICODE_STRING *name, WCHAR *buff NTSTATUS WINAPI NtQuerySystemEnvironmentValueEx( UNICODE_STRING *name, GUID *vendor, void *buffer, ULONG *retlen, ULONG *attrib ) { +#if defined(__linux__) || defined(__gnu_linux__) + int fd, rc; + size_t bytes, pos = 0; + ssize_t ssz; + char filename[128]; + char *cname; + struct stat sb = { 0 }; + + cname = (char *)malloc( name->Length ); + ntdll_wcstoumbs( name->Buffer, name->Length, cname, name->Length, FALSE ); + + snprintf( filename, sizeof(filename), "/sys/firmware/efi/efivars/%s-%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X", + cname, vendor->Data1, vendor->Data2, vendor->Data3, + vendor->Data4[0], vendor->Data4[1], vendor->Data4[2], vendor->Data4[3], + vendor->Data4[4], vendor->Data4[5], vendor->Data4[6], vendor->Data4[7]); + + fd = open( filename, O_RDONLY | O_NONBLOCK ); + if (fd < 0) + goto done; + + rc = fstat( fd, &sb ); + if ( rc < 0 || sb.st_size == 0 ) + goto done; + + if ( sb.st_size - 4 < *retlen ) + bytes = sb.st_size - 4; + else + bytes = *retlen; + +try_read_attributes: + ssz = read( fd, attrib, 4 ); + if ( ssz < 0 ) { + if ( errno == EAGAIN || errno == EINTR ) + goto try_read_attributes; + goto done; + } + + while ( pos < bytes ) { + ssz = read( fd, (char *) buffer + pos, bytes - pos ); + if ( ssz < 0 ) { + if ( errno == EAGAIN || errno == EINTR ) + continue; + goto done; + } + pos += ssz; + } + + close( fd ); + *retlen = ssz & 0xFFFFFFFF; + return STATUS_SUCCESS; + +done: + if ( fd >= 0 ) + close( fd ); + *retlen = 0; + return STATUS_UNSUCCESSFUL; +#else FIXME( "(%s, %s, %p, %p, %p), stub\n", debugstr_us(name), debugstr_guid(vendor), buffer, retlen, attrib ); return STATUS_NOT_IMPLEMENTED; +#endif } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6423
From: Grigory Vasilyev <h0tc0d3(a)gmail.com> --- dlls/ntdll/unix/system.c | 64 ++++++++++++++++++++++++++++++++++++++++ dlls/wow64/system.c | 3 ++ include/winternl.h | 22 ++++++++++++++ 3 files changed, 89 insertions(+) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index 0d3a4f403ef..80975e2c810 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -2794,6 +2794,70 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class, break; } + case SystemBootEnvironmentInformation: /* 90 */ + { + static volatile SYSTEM_BOOT_ENVIRONMENT_INFORMATION boot_info = { 0 }; + len = sizeof(boot_info); + if(size == len) + { +#if defined(__linux__) || defined(__gnu_linux__) + int fd; + ssize_t ssz; + struct stat stat_info = { 0 }; + char buffer[32]; + + if( boot_info.FirmwareType == FirmwareTypeUnknown ) + { + if( !stat( "/sys/firmware/efi", &stat_info ) ) + boot_info.FirmwareType = FirmwareTypeUefi; + else + boot_info.FirmwareType = FirmwareTypeBios; + } + + if( boot_info.BootIdentifier.Data1 == 0) + { + if ( !stat( "/etc/machine-id", &stat_info ) && stat_info.st_size >= 32 ) + { + fd = open( "/etc/machine-id", O_RDONLY | O_NONBLOCK ); + if ( fd >= 0 ) + { + try_read_guid: + ssz = read( fd, &buffer, 32 ); + if ( ssz < 0 && ( errno == EAGAIN || errno == EINTR ) ) + goto try_read_guid; + close( fd ); + sscanf( buffer, "%08lx%04hx%04hx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx", + &boot_info.BootIdentifier.Data1, &boot_info.BootIdentifier.Data2, + &boot_info.BootIdentifier.Data3, &boot_info.BootIdentifier.Data4[0], + &boot_info.BootIdentifier.Data4[1], &boot_info.BootIdentifier.Data4[2], + &boot_info.BootIdentifier.Data4[3], &boot_info.BootIdentifier.Data4[4], + &boot_info.BootIdentifier.Data4[5], &boot_info.BootIdentifier.Data4[6], + &boot_info.BootIdentifier.Data4[7] ); + } + else goto try_read_random; + } else { + try_read_random: + ssz = getrandom( &boot_info.BootIdentifier, sizeof( boot_info.BootIdentifier ), 0 ); + if ( ssz < 0 && ( errno == EAGAIN || errno == EINTR ) ) + goto try_read_random; + } + + boot_info.BootIdentifier.Data3 &= 0x0fff; + boot_info.BootIdentifier.Data3 |= (4 << 12); + /* Set the topmost bits of Data4 (clock_seq_hi_and_reserved) as + * specified in RFC 4122, section 4.4. + */ + boot_info.BootIdentifier.Data4[0] &= 0x3f; + boot_info.BootIdentifier.Data4[0] |= 0x80; + } +#endif + memcpy( info, &boot_info, len ); + } + else ret = STATUS_INFO_LENGTH_MISMATCH; + if ( ret_size ) *ret_size = len; + break; + } + case SystemCpuInformation: /* 1 */ if (size >= (len = sizeof(cpu_info))) memcpy(info, &cpu_info, len); else ret = STATUS_INFO_LENGTH_MISMATCH; diff --git a/dlls/wow64/system.c b/dlls/wow64/system.c index 5f3056a4179..3835f815072 100644 --- a/dlls/wow64/system.c +++ b/dlls/wow64/system.c @@ -356,6 +356,9 @@ NTSTATUS WINAPI wow64_NtQuerySystemInformation( UINT *args ) if (retlen) *retlen = sizeof(SYSTEM_BASIC_INFORMATION32); return status; + case SystemBootEnvironmentInformation: /* SYSTEM_BOOT_ENVIRONMENT_INFORMATION */ + return NtQuerySystemInformation( class, ptr, len, retlen ); + case SystemProcessInformation: /* SYSTEM_PROCESS_INFORMATION */ case SystemExtendedProcessInformation: /* SYSTEM_PROCESS_INFORMATION */ { diff --git a/include/winternl.h b/include/winternl.h index 828fbc6e915..510bba09042 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -2724,6 +2724,28 @@ typedef struct _SYSTEM_BASIC_INFORMATION { #endif } SYSTEM_BASIC_INFORMATION, *PSYSTEM_BASIC_INFORMATION; +/* System Information Class 0x90 */ +typedef struct _SYSTEM_BOOT_ENVIRONMENT_INFORMATION +{ + GUID BootIdentifier; + FIRMWARE_TYPE FirmwareType; + union + { + ULONGLONG BootFlags; + struct + { + ULONGLONG DbgMenuOsSelection : 1; + ULONGLONG DbgHiberBoot : 1; + ULONGLONG DbgSoftBoot : 1; + ULONGLONG DbgMeasuredLaunch : 1; + ULONGLONG DbgMeasuredLaunchCapable : 1; + ULONGLONG DbgSystemHiveReplace : 1; + ULONGLONG DbgMeasuredLaunchSmmProtections : 1; + ULONGLONG DbgMeasuredLaunchSmmLevel : 7; + }; + }; +} SYSTEM_BOOT_ENVIRONMENT_INFORMATION, *PSYSTEM_BOOT_ENVIRONMENT_INFORMATION; + /* System Information Class 0x01 */ typedef struct _SYSTEM_CPU_INFORMATION { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6423
From: Grigory Vasilyev <h0tc0d3(a)gmail.com> --- dlls/kernel32/process.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 2a4ddc68f02..4d27932a372 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -766,7 +766,10 @@ BOOL WINAPI GetFirmwareType(FIRMWARE_TYPE *type) if (!type) return FALSE; - *type = FirmwareTypeUnknown; + ULONG ret_size ; + SYSTEM_BOOT_ENVIRONMENT_INFORMATION boot_info = { 0 }; + NtQuerySystemInformation( SystemBootEnvironmentInformation, &boot_info, sizeof(boot_info), &ret_size ); + *type = boot_info.FirmwareType; return TRUE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6423
From: Grigory Vasilyev <h0tc0d3(a)gmail.com> --- dlls/kernel32/Makefile.in | 2 + dlls/kernel32/firmware.c | 122 ++++++++++++++++++++++++++++++++++++ dlls/kernel32/kernel32.spec | 2 + dlls/kernel32/kernel_main.c | 2 + dlls/kernel32/process.c | 58 +++++++++++++++-- dlls/kernel32/unixlib.h | 44 +++++++++++++ 6 files changed, 224 insertions(+), 6 deletions(-) create mode 100644 dlls/kernel32/firmware.c create mode 100644 dlls/kernel32/unixlib.h diff --git a/dlls/kernel32/Makefile.in b/dlls/kernel32/Makefile.in index ca5bb437e24..feb02d23e39 100644 --- a/dlls/kernel32/Makefile.in +++ b/dlls/kernel32/Makefile.in @@ -1,5 +1,6 @@ EXTRADEFS = -D_KERNEL32_ -D_NORMALIZE_ MODULE = kernel32.dll +UNIXLIB = kernel32.so IMPORTLIB = kernel32 IMPORTS = kernelbase ntdll winecrt0 @@ -14,6 +15,7 @@ SOURCES = \ console.c \ debugger.c \ file.c \ + firmware.c \ heap.c \ kernel_main.c \ locale.c \ diff --git a/dlls/kernel32/firmware.c b/dlls/kernel32/firmware.c new file mode 100644 index 00000000000..d00710faaf4 --- /dev/null +++ b/dlls/kernel32/firmware.c @@ -0,0 +1,122 @@ +/* + * Kernel32 Firmware Interface + * + * Copyright (C) 2024 Grigory Vasilyev + * + * 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 + */ + +#if 0 +#pragma makedep unix +#endif + +#include <errno.h> +#include <dirent.h> +#include <fcntl.h> +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <unistd.h> + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "unixlib.h" +#include "windef.h" + +/* Get Firmware Variable */ +static NTSTATUS kernel32_unix_get_firmware_variable(void *args) +{ + int fd, rc; + size_t bytes, pos = 0; + ssize_t ssz; + char tguid[37]; + char filename[128]; + struct stat sb = {0}; + struct wine_get_firmware_variable_params *params = args; + + int gsize = strlen(params->guid); + + if (params->guid[0] == '{' && params->guid[(gsize - 1)] == '}') { + if (gsize - 2 < sizeof(tguid)) + bytes = gsize - 1; + else + bytes = sizeof(tguid); + memcpy(tguid, params->guid + 1, bytes); + tguid[bytes - 1] = '\0'; + snprintf(filename, sizeof(filename), "/sys/firmware/efi/efivars/%s-%s", params->name, tguid); + } else { + snprintf(filename, sizeof(filename), "/sys/firmware/efi/efivars/%s-%s", params->name, params->guid); + } + + fd = open(filename, O_RDONLY | O_NONBLOCK); + if (fd < 0) + goto done; + + rc = fstat(fd, &sb); + if (rc < 0 || sb.st_size == 0) + goto done; + + if (sb.st_size - 4 < params->size) + bytes = sb.st_size - 4; + else + bytes = params->size; + +try_read_attributes: + ssz = read(fd, params->attributes, 4); + if (ssz < 0) { + if (errno == EAGAIN || errno == EINTR) + goto try_read_attributes; + goto done; + } + + while (pos < bytes) { + ssz = read(fd, (char *) params->buffer + pos, bytes - pos); + if (ssz < 0) { + if (errno == EAGAIN || errno == EINTR) + continue; + goto done; + } + pos += ssz; + } + + close(fd); + params->status = ssz & 0xFFFFFFFF; + return STATUS_SUCCESS; + +done: + if (fd >= 0) + close(fd); + params->status = 0; + return STATUS_UNSUCCESSFUL; +} + +const unixlib_entry_t __wine_unix_call_funcs[] = { + kernel32_unix_get_firmware_variable, +}; + +C_ASSERT(ARRAYSIZE(__wine_unix_call_funcs) == kernel32_unix_func_count); + +#ifdef _WIN64 + +const unixlib_entry_t __wine_unix_call_wow64_funcs[] = { + kernel32_unix_get_firmware_variable, +}; + +C_ASSERT(ARRAYSIZE(__wine_unix_call_wow64_funcs) == kernel32_unix_func_count); + +#endif /* _WIN64 */ diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index 7ed8048f5d0..5d3556a2078 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -695,7 +695,9 @@ @ stdcall -import GetFileType(long) @ stdcall -import GetFinalPathNameByHandleA(long ptr long long) @ stdcall -import GetFinalPathNameByHandleW(long ptr long long) +@ stdcall GetFirmwareEnvironmentVariableExA(str str ptr long ptr) @ stdcall GetFirmwareEnvironmentVariableA(str str ptr long) +@ stdcall GetFirmwareEnvironmentVariableExW(wstr wstr ptr long ptr) @ stdcall GetFirmwareEnvironmentVariableW(wstr wstr ptr long) @ stdcall GetFirmwareType(ptr) @ stdcall -import GetFullPathNameA(str long ptr ptr) diff --git a/dlls/kernel32/kernel_main.c b/dlls/kernel32/kernel_main.c index ae16195a550..70443cbce2a 100644 --- a/dlls/kernel32/kernel_main.c +++ b/dlls/kernel32/kernel_main.c @@ -31,6 +31,7 @@ #include "kernel_private.h" #include "wine/debug.h" +#include "wine/unixlib.h" WINE_DEFAULT_DEBUG_CHANNEL(process); @@ -161,6 +162,7 @@ BOOL WINAPI DllMain( HINSTANCE hinst, DWORD reason, LPVOID reserved ) { case DLL_PROCESS_ATTACH: DisableThreadLibraryCalls( hinst ); + if (__wine_init_unix_call()) return FALSE; return process_attach( hinst ); case DLL_PROCESS_DETACH: WritePrivateProfileSectionW( NULL, NULL, NULL ); diff --git a/dlls/kernel32/process.c b/dlls/kernel32/process.c index 4d27932a372..2da84d5b899 100644 --- a/dlls/kernel32/process.c +++ b/dlls/kernel32/process.c @@ -37,6 +37,7 @@ #include "ddk/wdm.h" #include "wine/asm.h" #include "wine/debug.h" +#include "unixlib.h" WINE_DEFAULT_DEBUG_CHANNEL(process); @@ -718,14 +719,60 @@ WORD WINAPI GetMaximumProcessorGroupCount(void) return groups; } +/*********************************************************************** + * GetFirmwareEnvironmentVariableExA (KERNEL32.@) + */ +DWORD WINAPI GetFirmwareEnvironmentVariableExA(LPCSTR name, LPCSTR guid, PVOID buffer, DWORD size, PDWORD attributes) +{ + struct wine_get_firmware_variable_params params = { 0 }; + params.name = name; + params.guid = guid; + params.buffer = buffer; + params.size = size; + params.attributes = attributes; + WINE_UNIX_CALL( kernel32_unix_func_get_firmware_variable, ¶ms ); + return params.status; +} + /*********************************************************************** * GetFirmwareEnvironmentVariableA (KERNEL32.@) */ DWORD WINAPI GetFirmwareEnvironmentVariableA(LPCSTR name, LPCSTR guid, PVOID buffer, DWORD size) { - FIXME("stub: %s %s %p %lu\n", debugstr_a(name), debugstr_a(guid), buffer, size); - SetLastError(ERROR_INVALID_FUNCTION); - return 0; + DWORD dwAttributes; + return GetFirmwareEnvironmentVariableExA( name, guid, buffer, size, &dwAttributes ); +} + +/*********************************************************************** + * GetFirmwareEnvironmentVariableExW (KERNEL32.@) + */ +DWORD WINAPI GetFirmwareEnvironmentVariableExW(LPCWSTR name, LPCWSTR guid, PVOID buffer, DWORD size, PDWORD attributes) +{ + DWORD ret; + INT len; + char *aname; + char *aguid; + + len = lstrlenW( name ) + 1; + ret = WideCharToMultiByte( GetOEMCP(), 0, name, len, NULL, 0, NULL, NULL ); + if ( !( aname = RtlAllocateHeap( GetProcessHeap(), 0, ret * sizeof(aname) ) ) ) + return 0; + WideCharToMultiByte( GetOEMCP(), 0, name, len, aname, ret, NULL, NULL ); + + len = lstrlenW( guid ) + 1; + ret = WideCharToMultiByte( GetOEMCP(), 0, guid, len, NULL, 0, NULL, NULL ); + if ( !( aguid = RtlAllocateHeap( GetProcessHeap(), 0, ret * sizeof(aguid) ) ) ) { + HeapFree( GetProcessHeap(), 0, aname ); + return 0; + } + WideCharToMultiByte( GetOEMCP(), 0, guid, len, aguid, ret, NULL, NULL ); + + ret = GetFirmwareEnvironmentVariableExA( aname, aguid, buffer, size, attributes ); + + HeapFree( GetProcessHeap(), 0, aname ); + HeapFree( GetProcessHeap(), 0, aguid ); + + return ret; } /*********************************************************************** @@ -733,9 +780,8 @@ DWORD WINAPI GetFirmwareEnvironmentVariableA(LPCSTR name, LPCSTR guid, PVOID buf */ DWORD WINAPI GetFirmwareEnvironmentVariableW(LPCWSTR name, LPCWSTR guid, PVOID buffer, DWORD size) { - FIXME("stub: %s %s %p %lu\n", debugstr_w(name), debugstr_w(guid), buffer, size); - SetLastError(ERROR_INVALID_FUNCTION); - return 0; + DWORD dwAttributes; + return GetFirmwareEnvironmentVariableExW( name, guid, buffer, size, &dwAttributes ); } /*********************************************************************** diff --git a/dlls/kernel32/unixlib.h b/dlls/kernel32/unixlib.h new file mode 100644 index 00000000000..1f6b7b409d9 --- /dev/null +++ b/dlls/kernel32/unixlib.h @@ -0,0 +1,44 @@ +/* + * Kernel32 Unix interface + * + * Copyright (C) 2024 Grigory Vasilyev + * + * 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 + */ + +#ifndef __WINE_KERNEL_UNIXLIB_H +#define __WINE_KERNEL_UNIXLIB_H + +#include "wine/unixlib.h" + +struct wine_get_firmware_variable_params +{ + LPCSTR name; + LPCSTR guid; + PVOID buffer; + DWORD size; + PDWORD attributes; + DWORD status; +}; + +enum kernel32_unix_func +{ + kernel32_unix_func_get_firmware_variable, + kernel32_unix_func_count, +}; + +extern unixlib_handle_t __wine_unixlib_handle; + +#endif /* __WINE_KERNEL_UNIXLIB_H */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6423
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=148225 Your paranoid android. === debian11 (build log) === /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/winecrt0/unix_lib.c:52: undefined reference to `_imp__LdrGetDllHandle(a)16' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/winecrt0/unix_lib.c:53: undefined reference to `_imp__RtlFindExportedRoutineByName(a)8' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/winecrt0/unix_lib.c:94: undefined reference to `_imp__NtQueryVirtualMemory(a)24' collect2: error: ld returned 1 exit status Task: The win32 Wine build failed === debian11b (build log) === /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/winecrt0/unix_lib.c:52: undefined reference to `__imp_LdrGetDllHandle' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/winecrt0/unix_lib.c:53: undefined reference to `__imp_RtlFindExportedRoutineByName' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/winecrt0/unix_lib.c:94: undefined reference to `__imp_NtQueryVirtualMemory' collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
Nikolay Sivov (@nsivov) commented about dlls/kernel32/process.c:
if (!type) return FALSE;
- *type = FirmwareTypeUnknown; + ULONG ret_size ; + SYSTEM_BOOT_ENVIRONMENT_INFORMATION boot_info = { 0 }; + NtQuerySystemInformation( SystemBootEnvironmentInformation, &boot_info, sizeof(boot_info), &ret_size ); + *type = boot_info.FirmwareType; return TRUE; }
Usual pattern is to use set_ntstatus(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81127
Nikolay Sivov (@nsivov) commented about dlls/kernel32/Makefile.in:
EXTRADEFS = -D_KERNEL32_ -D_NORMALIZE_ MODULE = kernel32.dll +UNIXLIB = kernel32.so IMPORTLIB = kernel32 IMPORTS = kernelbase ntdll winecrt0 This has to be removed, you'll be calling ntdll function for this.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81128
Nikolay Sivov (@nsivov) commented about dlls/kernel32/process.c:
/*********************************************************************** * GetFirmwareEnvironmentVariableA (KERNEL32.@) */ DWORD WINAPI GetFirmwareEnvironmentVariableA(LPCSTR name, LPCSTR guid, PVOID buffer, DWORD size) { - FIXME("stub: %s %s %p %lu\n", debugstr_a(name), debugstr_a(guid), buffer, size); - SetLastError(ERROR_INVALID_FUNCTION); - return 0; + DWORD dwAttributes; + return GetFirmwareEnvironmentVariableExA( name, guid, buffer, size, &dwAttributes ); +} + +/*********************************************************************** + * GetFirmwareEnvironmentVariableExW (KERNEL32.@) + */ +DWORD WINAPI GetFirmwareEnvironmentVariableExW(LPCWSTR name, LPCWSTR guid, PVOID buffer, DWORD size, PDWORD attributes) It should be the other way around. ExW calls Nt function that accepts UNICODE_STRING or WCHARs, and ExA converts acp->wchar and calls ExW function. Assuming returned buffer contents are the same for both functions of course.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81129
On Wed Sep 4 20:57:34 2024 +0000, Nikolay Sivov wrote:
Usual pattern is to use set_ntstatus(). @nsivov I don't quite understand you. If NtQuerySystemInformation fails then boot_info.FirmwareType will contain FirmwareTypeUnknown, which is consistent with previous behavior, and will also be on systems other than Linux. Thus, it does not matter what status NtQuerySystemInformation returns, we will always have the current value corresponding to our system.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81145
On Wed Sep 4 21:55:51 2024 +0000, Grigory Vasilyev wrote:
@nsivov I don't quite understand you. If NtQuerySystemInformation fails then boot_info.FirmwareType will contain FirmwareTypeUnknown, which is consistent with previous behavior, and will also be on systems other than Linux. Thus, it does not matter what status NtQuerySystemInformation returns, we will always have the current value corresponding to our system. Take a look at how it's used for other functions.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81146
On Wed Sep 4 22:10:28 2024 +0000, Nikolay Sivov wrote:
Take a look at how it's used for other functions. @nsivov I still don't understand why we need to set last error code if we always return TRUE and we will always have a valid value. Look at the code, it doesn't matter whether `NtQuerySystemInformation` returns an error or not. There will always be a default value of `FirmwareTypeUnknown` or whatever `NtQuerySystemInformation` returns. This is done on purpose so that the code will continue to work as before on systems other than Linux.
``` static inline BOOL set_ntstatus( NTSTATUS status ) { if (status) SetLastError( RtlNtStatusToDosError( status )); return !status; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81148
On Wed Sep 4 23:32:23 2024 +0000, Grigory Vasilyev wrote:
@nsivov I still don't understand why we need to set last error code if we always return TRUE and we will always have a valid value. Look at the code, it doesn't matter whether `NtQuerySystemInformation` returns an error or not. There will always be a default value of `FirmwareTypeUnknown` or whatever `NtQuerySystemInformation` returns. This is done on purpose so that the code will continue to work as before on systems other than Linux. ``` static inline BOOL set_ntstatus( NTSTATUS status ) { if (status) SetLastError( RtlNtStatusToDosError( status )); return !status; } ``` If you're going to have a fallback, it should be in NtQuerySystemInformation.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81153
Alfred Agrell (@Alcaro) commented about dlls/kernel32/firmware.c:
+ char filename[128]; + struct stat sb = {0}; + struct wine_get_firmware_variable_params *params = args; + + int gsize = strlen(params->guid); + + if (params->guid[0] == '{' && params->guid[(gsize - 1)] == '}') { + if (gsize - 2 < sizeof(tguid)) + bytes = gsize - 1; + else + bytes = sizeof(tguid); + memcpy(tguid, params->guid + 1, bytes); + tguid[bytes - 1] = '\0'; + snprintf(filename, sizeof(filename), "/sys/firmware/efi/efivars/%s-%s", params->name, tguid); + } else { + snprintf(filename, sizeof(filename), "/sys/firmware/efi/efivars/%s-%s", params->name, params->guid); This is inaccurate, native returns ERROR_INVALID_PARAMETER if the {} is missing. (At least on my 10 VM, didn't check 11.)
(Conveniently, this is also what RtlGUIDFromString does.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81173
Alfred Agrell (@Alcaro) commented about dlls/kernel32/process.c:
if (!type) return FALSE;
- *type = FirmwareTypeUnknown; + ULONG ret_size ;
You sure you want a space before that semicolon? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81186
On Thu Sep 5 11:35:12 2024 +0000, Alfred Agrell wrote:
You sure you want a space before that semicolon? I'm pretty sure this was their mistake
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81188
On Thu Sep 5 03:57:23 2024 +0000, Elizabeth Figura wrote:
If you're going to have a fallback, it should be in NtQuerySystemInformation. Yes, what I would do probably is "if (!set_ntstatus()) return FALSE; *type = ...; return TRUE;". We can't really test a case when nt call fails, so it's fine to assume it doesn't.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81194
On Thu Sep 5 12:00:12 2024 +0000, Aida Jonikienė wrote:
I'm pretty sure this was their mistake Normally patches adding functions (like in fact almost any patches) can use some unit tests included, any reason tests are not needed here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6423#note_81203
participants (8)
-
Aida Jonikienė -
Alfred Agrell (＠Alcaro)
-
Elizabeth Figura (＠zfigura)
-
Grigory Vasilyev
-
Grigory Vasilyev (＠h0tc0d3)
-
Marvin -
Nikolay Sivov (＠nsivov)
-
Paul Gofman (＠gofman)