[PATCH v4 0/9] MR5769: Add support for large page backed file mapping objects on Linux.
This MR adds support for creating file mapping objects backed by large pages on Linux, by making the following changes: ## wineserver * On Linux, `create_temp_file` will first attempt to use memfds as the backing fd. If it fails, it'll return to the current codepath, creating a temporary file in either the server or config directory. * The created memfd will be sealed against writes, if the caller requesting the appropriate page protection flags. * This removes the requirement that FDs be only created on filesystems/directories that aren't `noexec`. * In the server method `create_mapping` , if large pages have been requested by the caller, hold that the calling thread's token holds `SeLockMemoryPrivilege` . * Additionally, add `SeLockMemoryPrivilege` to the list of privileges enabled for the Administrator. ## `ntdll` * Add `virtual_get_min_large_page_size` and its exported wrapper `wine_unix_get_min_large_page_size`. * On Linux, the minimum page size is determined by going through `/sys/kernel/mm/hugepages`. If hugepage support was not detected, `STATUS_NOT_SUPPORTED` is returned instead. On other platforms, the older hard-coded value of 2\*1024\*1024 is returned instead. * `NtCreateSection` will validate certain parameters if large pages are requested. Specifically, it will return STATUS_INVALID_PARAMETER if the requested mapping is not anonymous/unnamed, or the size is not a multiple of the minimum supported page size. ## `kernelbase` * `GetLargePageMinimum` will use `wine_unix_get_min_large_page_size`. ## `kernel32/tests` * Add new test test_large_page_file_mapping, which validates privilege enforcements and parameter validation while creating large pages backed file mapping obejcts. The tests are skipped if `GetLargePageMinimum` returns 0. -- v4: virtual_get_min_large_page_size: initialize min_size to 0 https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> --- server/mapping.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/server/mapping.c b/server/mapping.c index ff99b45ce51..2280360f6cd 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -21,6 +21,7 @@ #include "config.h" #include <assert.h> +#include <errno.h> #include <fcntl.h> #include <stdarg.h> #include <stdio.h> @@ -317,13 +318,90 @@ static int check_current_dir_for_exec(void) return (ret != MAP_FAILED); } +#if defined( __linux__ ) && defined( _GNU_SOURCE ) +static void make_memfd_name( const char *prefix, ULONG sec_flags, char *dest ) +{ + /* memfd names are restricted to 250 bytes including \0, but are only meant for debugging, with + * duplicate names not causing any side-effects. */ + + if (prefix != NULL) + { + snprintf( dest, 250, "map-%s-%lx-%lld", prefix, sec_flags, current_time); + } + else + { + snprintf( dest, 250, "map-%lx-%lld", sec_flags, current_time ); + } +} + +static int create_memfd( ULONG file_access, ULONG sec_flags, file_pos_t size ) +{ + int fd; + char memfd_name[256]; + unsigned int memfd_flags = MFD_ALLOW_SEALING; + unsigned int seal_flags = F_SEAL_SEAL | F_SEAL_WRITE; + + make_memfd_name( NULL, sec_flags, memfd_name ); + + if (sec_flags & SEC_LARGE_PAGES) + { + memfd_flags |= MFD_HUGETLB; + } + fd = memfd_create( memfd_name, memfd_flags ); + if (fd == -1) + { + if (errno == EINVAL && (sec_flags & SEC_LARGE_PAGES)) + { + /* MFD_HUGETLB & MFD_ALLOW_SEALING is only available in Linux >= 4.16. Lets try creating + * one without HUGETLB */ + fd = memfd_create( memfd_name, memfd_flags & ~MFD_HUGETLB ); + if (fd == -1) + { + file_set_error(); + return -1; + } + } + else + { + return -1; + } + } + if (ftruncate(fd, size) == -1) + { + file_set_error(); + close( fd ); + return -1; + } + if (file_access & FILE_WRITE_DATA) + { + seal_flags &= ~F_SEAL_WRITE; + } + + if (fcntl( fd, F_ADD_SEALS, F_SEAL_SEAL, seal_flags ) == -1) + { + file_set_error(); + close( fd ); + return -1; + } + return fd; +} +#endif /* defined( __linux__ ) && defined( _GNU_SOURCE ) */ + /* create a temp file for anonymous mappings */ -static int create_temp_file( file_pos_t size ) +static int create_temp_file( ULONG file_access, ULONG sec_flags, file_pos_t size ) { static int temp_dir_fd = -1; char tmpfn[16]; int fd; +#if defined( __linux__ ) && defined( _GNU_SOURCE ) + fd = create_memfd( file_access, sec_flags, size ); + if (fd != -1) + { + return fd; + } +#endif + if (temp_dir_fd == -1) { temp_dir_fd = server_dir_fd; @@ -608,7 +686,7 @@ static int build_shared_mapping( struct mapping *mapping, int fd, /* create a temp file for the mapping */ - if ((shared_fd = create_temp_file( total_size )) == -1) return 0; + if ((shared_fd = create_temp_file( FILE_WRITE_DATA, 0, total_size )) == -1) return 0; if (!(file = create_file_for_fd( shared_fd, FILE_GENERIC_READ|FILE_GENERIC_WRITE, 0 ))) return 0; if (!(buffer = malloc( max_size ))) goto error; @@ -1044,7 +1122,7 @@ static struct mapping *create_mapping( struct object *root, const struct unicode } if ((flags & SEC_RESERVE) && !(mapping->committed = create_ranges())) goto error; mapping->size = (mapping->size + page_mask) & ~((mem_size_t)page_mask); - if ((unix_fd = create_temp_file( mapping->size )) == -1) goto error; + if ((unix_fd = create_temp_file( mapping->size, file_access, flags )) == -1) goto error; if (!(mapping->fd = create_anonymous_fd( &mapping_fd_ops, unix_fd, &mapping->obj, FILE_SYNCHRONOUS_IO_NONALERT ))) goto error; allow_fd_caching( mapping->fd ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> --- server/security.h | 1 + server/token.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/server/security.h b/server/security.h index 58ab1594eae..6cf14a44dfe 100644 --- a/server/security.h +++ b/server/security.h @@ -23,6 +23,7 @@ #include <sys/types.h> +extern const struct luid SeLockMemoryPrivilege; extern const struct luid SeIncreaseQuotaPrivilege; extern const struct luid SeSecurityPrivilege; extern const struct luid SeTakeOwnershipPrivilege; diff --git a/server/token.c b/server/token.c index 4df8d2e0c6e..3260c516956 100644 --- a/server/token.c +++ b/server/token.c @@ -42,6 +42,7 @@ #define MAX_SUBAUTH_COUNT 1 +const struct luid SeLockMemoryPrivilege = { 4, 0}; const struct luid SeIncreaseQuotaPrivilege = { 5, 0 }; const struct luid SeTcbPrivilege = { 7, 0 }; const struct luid SeSecurityPrivilege = { 8, 0 }; @@ -756,6 +757,7 @@ struct token *token_create_admin( unsigned primary, int impersonation_level, int { SeManageVolumePrivilege, 0 }, { SeImpersonatePrivilege, SE_PRIVILEGE_ENABLED }, { SeCreateGlobalPrivilege, SE_PRIVILEGE_ENABLED }, + { SeLockMemoryPrivilege, SE_PRIVILEGE_ENABLED }, }; /* note: we don't include non-builtin groups here for the user - * telling us these is the job of a client-side program */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> On Linux (for now), virtual_get_min_large_page_size will iterate through /sys/kernel/mm/hugepages and return the minimum huge page size supported by the system. The exported function wine_unix_get_min_large_page_size is a wrapper around this function. --- dlls/ntdll/ntdll.spec | 3 ++ dlls/ntdll/ntsyscalls.h | 6 ++- dlls/ntdll/unix/unix_private.h | 1 + dlls/ntdll/unix/virtual.c | 74 ++++++++++++++++++++++++++++++++++ include/winternl.h | 2 + 5 files changed, 84 insertions(+), 2 deletions(-) diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec index 3471905c762..07fc0150926 100644 --- a/dlls/ntdll/ntdll.spec +++ b/dlls/ntdll/ntdll.spec @@ -1741,3 +1741,6 @@ # Filesystem @ stdcall -syscall wine_nt_to_unix_file_name(ptr ptr ptr long) @ stdcall -syscall wine_unix_to_nt_file_name(str ptr ptr) + +# Memory management +@ stdcall -syscall wine_unix_get_min_large_page_size(ptr) diff --git a/dlls/ntdll/ntsyscalls.h b/dlls/ntdll/ntsyscalls.h index 55e1436848b..8f7619a4866 100644 --- a/dlls/ntdll/ntsyscalls.h +++ b/dlls/ntdll/ntsyscalls.h @@ -242,7 +242,8 @@ SYSCALL_ENTRY( 0x00ee, NtWriteVirtualMemory, 20 ) \ SYSCALL_ENTRY( 0x00ef, NtYieldExecution, 0 ) \ SYSCALL_ENTRY( 0x00f0, wine_nt_to_unix_file_name, 16 ) \ - SYSCALL_ENTRY( 0x00f1, wine_unix_to_nt_file_name, 12 ) + SYSCALL_ENTRY( 0x00f1, wine_unix_get_min_large_page_size, 4 ) \ + SYSCALL_ENTRY( 0x00f2, wine_unix_to_nt_file_name, 12 ) #define ALL_SYSCALLS64 \ SYSCALL_ENTRY( 0x0000, NtAcceptConnectPort, 48 ) \ @@ -481,4 +482,5 @@ SYSCALL_ENTRY( 0x00e9, NtWriteVirtualMemory, 40 ) \ SYSCALL_ENTRY( 0x00ea, NtYieldExecution, 0 ) \ SYSCALL_ENTRY( 0x00eb, wine_nt_to_unix_file_name, 32 ) \ - SYSCALL_ENTRY( 0x00ec, wine_unix_to_nt_file_name, 24 ) + SYSCALL_ENTRY( 0x00ec, wine_unix_get_min_large_page_size, 8 ) \ + SYSCALL_ENTRY( 0x00ed, wine_unix_to_nt_file_name, 24 ) diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index b278ab8df84..80b366f8b74 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -256,6 +256,7 @@ extern void *anon_mmap_alloc( size_t size, int prot ); extern void virtual_init(void); extern ULONG_PTR get_system_affinity_mask(void); extern void virtual_get_system_info( SYSTEM_BASIC_INFORMATION *info, BOOL wow64 ); +extern NTSTATUS virtual_get_min_large_page_size( SIZE_T *size ); extern NTSTATUS virtual_map_builtin_module( HANDLE mapping, void **module, SIZE_T *size, SECTION_IMAGE_INFORMATION *info, ULONG_PTR limit_low, ULONG_PTR limit_high, WORD machine, BOOL prefer_native ); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 4b23d9954df..d5c968866b8 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -63,6 +63,10 @@ # include <mach/mach_init.h> # include <mach/mach_vm.h> #endif +#ifdef __linux__ +#include <sys/types.h> +#include <dirent.h> +#endif #include "ntstatus.h" #define WIN32_NO_STATUS @@ -3394,6 +3398,76 @@ void virtual_get_system_info( SYSTEM_BASIC_INFORMATION *info, BOOL wow64 ) else info->HighestUserAddress = (char *)user_space_limit - 1; } +NTSTATUS WINAPI wine_unix_get_min_large_page_size( SIZE_T *size ) +{ + return virtual_get_min_large_page_size( size ); +} + +NTSTATUS virtual_get_min_large_page_size( SIZE_T *size ) +{ +#ifdef __linux__ + DIR *sysfs_hugepages; + struct dirent *supported_size; + SIZE_T min_size; + SIZE_T total_supported_sizes = 0; +#endif + + if (size == NULL) + { + return STATUS_INVALID_PARAMETER; + } + +#ifdef __linux__ + errno = 0; + sysfs_hugepages = opendir( "/sys/kernel/mm/hugepages" ); + if (sysfs_hugepages == NULL) + { + switch (errno) + { + case ENOENT: /* No huge pages support */ + return STATUS_NOT_SUPPORTED; + case ENOMEM: + return STATUS_NO_MEMORY; + default: + return STATUS_INTERNAL_ERROR; + } + } + supported_size = readdir( sysfs_hugepages ); + while (supported_size != NULL) + { + SIZE_T page_size; + + /* Entries are of the form "hugepages-${size}kB" */ + if (strncmp( supported_size->d_name, "hugepages-", 10 ) != 0) + { + supported_size = readdir( sysfs_hugepages ); + continue; + } + page_size = strtol( &supported_size->d_name[10], NULL, 10 ); + if (page_size == 0 || page_size == LONG_MAX || page_size == LONG_MIN) + { + ERR( "could not parse page size from /sys/kernel/mm/hugepages entry %s\n", + supported_size->d_name ); + supported_size = readdir( sysfs_hugepages ); + continue; + } + page_size *= 1024; + min_size = ( total_supported_sizes == 0 ) ? page_size + : ( page_size < min_size ? page_size : min_size ); + total_supported_sizes++; + supported_size = readdir( sysfs_hugepages ); + } + + closedir(sysfs_hugepages); + if (total_supported_sizes == 0) + return STATUS_NOT_SUPPORTED; + *size = min_size; +#else /* __linux__ */ + *size = 2 * 1024 * 1024; +#endif /* !defined(__linux) */ + return STATUS_SUCCESS; +} + /*********************************************************************** * virtual_map_builtin_module diff --git a/include/winternl.h b/include/winternl.h index 22cdceeaedb..72bdb2b1673 100644 --- a/include/winternl.h +++ b/include/winternl.h @@ -5148,6 +5148,8 @@ NTSYSAPI NTSTATUS WINAPI wine_nt_to_unix_file_name( const OBJECT_ATTRIBUTES *att UINT disposition ); NTSYSAPI NTSTATUS WINAPI wine_unix_to_nt_file_name( const char *name, WCHAR *buffer, ULONG *size ); +NTSYSAPI NTSTATUS WINAPI wine_unix_get_min_large_page_size( SIZE_T *size ); + /*********************************************************************** * Inline functions -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> Instead of returning a hardcoded value, use the newly added function wine_unix_get_min_large_page_size to get the minimum supported large page size, returning 0 if large pages are not supported on the host platform. --- dlls/kernelbase/memory.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c index 4f4bba9a13b..b4efa765c15 100644 --- a/dlls/kernelbase/memory.c +++ b/dlls/kernelbase/memory.c @@ -77,7 +77,11 @@ BOOL WINAPI DECLSPEC_HOTPATCH FlushViewOfFile( const void *base, SIZE_T size ) */ SIZE_T WINAPI GetLargePageMinimum(void) { - return 2 * 1024 * 1024; + NTSTATUS status; + SIZE_T min_size; + + status = wine_unix_get_min_large_page_size( &min_size ); + return status == STATUS_SUCCESS ? min_size : 0; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> In create_mapping, if the caller requires that the file be backed up by large pages, ensure that they hold the SeLockMemoryPrivilege. --- server/mapping.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/mapping.c b/server/mapping.c index 2280360f6cd..6b239e11807 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -1362,6 +1362,12 @@ DECL_HANDLER(create_mapping) if (!objattr) return; + if ((req->flags & SEC_LARGE_PAGES) && !thread_single_check_privilege( current, SeLockMemoryPrivilege )) + { + set_error( STATUS_PRIVILEGE_NOT_HELD ); + return; + } + if ((mapping = create_mapping( root, &name, objattr->attributes, req->size, req->flags, req->file_handle, req->file_access, sd ))) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> If the file mapping flags have SEC_LARGE_PAGES enabled, ensure that the mapping is anonymous and the requested size is a multiple of the minimum large page size supported by the host platform. --- dlls/ntdll/unix/sync.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index e070233104b..3727ff28acb 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2017,6 +2017,26 @@ NTSTATUS WINAPI NtCreateSection( HANDLE *handle, ACCESS_MASK access, const OBJEC *handle = 0; + if (sec_flags & SEC_LARGE_PAGES) + { + SIZE_T min_size; + NTSTATUS status; + + if (file != NULL || size == NULL) + { + return STATUS_INVALID_PARAMETER; + } + + status = virtual_get_min_large_page_size( &min_size ); + if (status != STATUS_SUCCESS) + { + return status == STATUS_NOT_SUPPORTED ? STATUS_INVALID_PARAMETER : status; + } + if (size->QuadPart % min_size != 0) + { + return STATUS_INVALID_PARAMETER; + } + } switch (protect & 0xff) { case PAGE_READONLY: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> The test will only run if the platform has a non-zero minimum large page size, and will check whether the correct privilege checks are enforced if large page backed mappings are requested, and whether only mapping sizes that are multiples of the minimum large page size are accepted. --- dlls/kernel32/tests/process.c | 64 +++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 8e168622c17..c0916bc60f0 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -82,6 +82,7 @@ static BOOL (WINAPI *pGetNumaProcessorNode)(UCHAR, PUCHAR); static NTSTATUS (WINAPI *pNtQueryInformationProcess)(HANDLE, PROCESSINFOCLASS, PVOID, ULONG, PULONG); static NTSTATUS (WINAPI *pNtQueryInformationThread)(HANDLE, THREADINFOCLASS, PVOID, ULONG, PULONG); static NTSTATUS (WINAPI *pNtQuerySystemInformationEx)(SYSTEM_INFORMATION_CLASS, void*, ULONG, void*, ULONG, ULONG*); +static NTSTATUS (WINAPI *pRtlAdjustPrivilege)(ULONG,BOOLEAN,BOOLEAN,PBOOLEAN); static DWORD (WINAPI *pWTSGetActiveConsoleSessionId)(void); static HANDLE (WINAPI *pCreateToolhelp32Snapshot)(DWORD, DWORD); static BOOL (WINAPI *pProcess32First)(HANDLE, PROCESSENTRY32*); @@ -252,6 +253,7 @@ static BOOL init(void) pNtQueryInformationProcess = (void *)GetProcAddress(hntdll, "NtQueryInformationProcess"); pNtQueryInformationThread = (void *)GetProcAddress(hntdll, "NtQueryInformationThread"); pNtQuerySystemInformationEx = (void *)GetProcAddress(hntdll, "NtQuerySystemInformationEx"); + pRtlAdjustPrivilege = (void *)GetProcAddress(hntdll, "RtlAdjustPrivilege"); pGetNativeSystemInfo = (void *) GetProcAddress(hkernel32, "GetNativeSystemInfo"); pGetSystemRegistryQuota = (void *) GetProcAddress(hkernel32, "GetSystemRegistryQuota"); @@ -4347,6 +4349,67 @@ static void test_largepages(void) ok((size == 0) || (size == 2*1024*1024) || (size == 4*1024*1024), "GetLargePageMinimum reports %Id size\n", size); } +static void test_large_page_file_mapping( void ) +{ + SIZE_T size; + BOOLEAN enabled; + NTSTATUS status; + DWORD err; + HANDLE file; + HANDLE token; + HANDLE process_token; + + if (!pGetLargePageMinimum) + { + win_skip( "No GetLargePageMinimum support.\n" ); + return; + } + if (!pRtlAdjustPrivilege) + { + win_skip( "No RtlAdjustPrivilege support.\n" ); + return; + } + + size = pGetLargePageMinimum(); + if (size == 0) + { + trace( "No large pages support, skipping test.\n" ); + return; + } + ok( OpenProcessToken( GetCurrentProcess(), TOKEN_DUPLICATE | TOKEN_QUERY, &process_token ), + "OpenProcessToken failed (%ld)\n", GetLastError() ); + ok( DuplicateToken( process_token, SecurityImpersonation, &token ), + "DuplicateToken failed (%ld)\n", GetLastError() ); + ok( ImpersonateLoggedOnUser( token ), "ImpersonateLoggedOnUser failed (%ld)\n", + GetLastError() ); + status = pRtlAdjustPrivilege( SE_LOCK_MEMORY_PRIVILEGE, TRUE, TRUE, &enabled ); + if (status != STATUS_SUCCESS) + { + trace( "Couldn't get SE_LOCK_MEMORY_PRIVILEGE (%ld), skipping large page file " + "mapping test.\n", + status ); + } + else + { + file = + CreateFileMappingW( INVALID_HANDLE_VALUE, NULL, + PAGE_READWRITE | SEC_LARGE_PAGES | SEC_COMMIT, 0, size - 1, NULL ); + err = GetLastError(); + ok( file == NULL && err == ERROR_INVALID_PARAMETER, + "CreateFileMappingW should fail with ERROR_INVALID_PARAMETER (got %ld instead)\n", + err ); + if (file != NULL) CloseHandle( file ); + + file = + CreateFileMappingW( INVALID_HANDLE_VALUE, NULL, + PAGE_READWRITE | SEC_LARGE_PAGES | SEC_COMMIT, 0, size * 2, NULL ); + ok( file != NULL, "CreateFileMappingW failed (%ld)\n", GetLastError() ); + if ( file != NULL ) CloseHandle( file ); + } + + ok( RevertToSelf(), "RevertToSelf failed (%ld)\n", GetLastError() ); +} + struct proc_thread_attr { DWORD_PTR attr; @@ -5668,6 +5731,7 @@ START_TEST(process) test_GetLogicalProcessorInformationEx(); test_GetSystemCpuSetInformation(); test_largepages(); + test_large_page_file_mapping(); test_ProcThreadAttributeList(); test_SuspendProcessState(); test_SuspendProcessNewThread(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> --- server/mapping.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/server/mapping.c b/server/mapping.c index 6b239e11807..a2958629c4d 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -30,6 +30,9 @@ #include <sys/stat.h> #include <sys/mman.h> #include <unistd.h> +#ifdef __linux__ +#include <linux/memfd.h> +#endif #include "ntstatus.h" #define WIN32_NO_STATUS @@ -341,6 +344,10 @@ static int create_memfd( ULONG file_access, ULONG sec_flags, file_pos_t size ) unsigned int memfd_flags = MFD_ALLOW_SEALING; unsigned int seal_flags = F_SEAL_SEAL | F_SEAL_WRITE; +#ifdef MFD_EXEC + memfd_flags |= MFD_EXEC; +#endif + make_memfd_name( NULL, sec_flags, memfd_name ); if (sec_flags & SEC_LARGE_PAGES) @@ -357,7 +364,6 @@ static int create_memfd( ULONG file_access, ULONG sec_flags, file_pos_t size ) fd = memfd_create( memfd_name, memfd_flags & ~MFD_HUGETLB ); if (fd == -1) { - file_set_error(); return -1; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
From: Vibhav Pant <vibhavp(a)gmail.com> --- dlls/ntdll/unix/virtual.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index d5c968866b8..0d8f905e578 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3408,7 +3408,7 @@ NTSTATUS virtual_get_min_large_page_size( SIZE_T *size ) #ifdef __linux__ DIR *sysfs_hugepages; struct dirent *supported_size; - SIZE_T min_size; + SIZE_T min_size = 0; SIZE_T total_supported_sizes = 0; #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5769
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=145986 Your paranoid android. === w1064v1507 (32 bit report) === kernel32: process.c:2741: Test failed: expected 0 assigned processes, got 1 process.c:2741: Test failed: expected 0 process IDs, got 1 === w1064v1809 (32 bit report) === kernel32: process.c:2741: Test failed: expected 0 assigned processes, got 1 process.c:2741: Test failed: expected 0 process IDs, got 1 === w10pro64_ar (64 bit report) === kernel32: process.c:2741: Test failed: expected 0 assigned processes, got 1 process.c:2741: Test failed: expected 0 process IDs, got 1 process.c:2742: Test failed: expected 0 active processes, got 1 === w10pro64_zh_CN (64 bit report) === kernel32: process.c:2741: Test failed: expected 0 assigned processes, got 1 process.c:2741: Test failed: expected 0 process IDs, got 1 === debian11b (build log) === /usr/bin/x86_64-w64-mingw32-ld: dlls/wow64/x86_64-windows/syscall.o:syscall.c:(.rdata+0xda8): undefined reference to `wow64_wine_unix_get_min_large_page_size' collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
Paul Gofman (@gofman) commented about dlls/ntdll/ntdll.spec:
# Filesystem @ stdcall -syscall wine_nt_to_unix_file_name(ptr ptr ptr long) @ stdcall -syscall wine_unix_to_nt_file_name(str ptr ptr) + +# Memory management +@ stdcall -syscall wine_unix_get_min_large_page_size(ptr) This shouldn't be added as a custom Wine syscall. There is at least LargePageMinimum in USER_SHARED_DATA which can be tested and filled. There also might be a way to get that through some NtQuerySystemInformation() although I don't know offhand if there is or what information class it can be. user_shared_data is already used in kernelbase so I guess using LargePageMinimum should be fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5769#note_71943
Paul Gofman (@gofman) commented about server/mapping.c:
+static void make_memfd_name( const char *prefix, ULONG sec_flags, char *dest ) +{ + /* memfd names are restricted to 250 bytes including \0, but are only meant for debugging, with + * duplicate names not causing any side-effects. */ + + if (prefix != NULL) + { + snprintf( dest, 250, "map-%s-%lx-%lld", prefix, sec_flags, current_time); + } + else + { + snprintf( dest, 250, "map-%lx-%lld", sec_flags, current_time ); + } +} + +static int create_memfd( ULONG file_access, ULONG sec_flags, file_pos_t size ) What is the motivation under introducing Linux specific memfd, is there any clear indication of how that is beneficial? Maybe there is something, but backing any file with memory fd doesn't seem obviously beneficial: it can probably provoke OOMs which otherwise would be avoided with file backed memory. Huge file backed mappings may be accessed very sparsely and not impose much RAM pressure.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5769#note_71944
Out of curiousity, which app needs that? Note that complete implementation will require much more than this, in the first place, proper support of large pages in VirtualProtect() and VirtualQuery(). I am not sure that the implementation of that is necessary at once, but maybe worth adding at least some very basic tests for that to see how bad that will broken without implementation? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5769#note_71945
Regarding the value for LargePageMinimum, why should it be attempted to determined from Linux somehow? Linux doesn't give much of explicit control for large pages, and we should care more about what Windows gives here as that what apps will expect. I guess we'd be better off with testing the value on Windows (which is probably just constant?) and use that one. Then, how exactly to back that with actual THPs is beyond our control (we can only hint with madvise whether to use huge pages or not). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5769#note_71946
PS in the end, at least if we will support proper _WRITECOPY reporting in NtQuerySystemInformation (which is not yet supported for normal pages as well) I am afraid we will actually have to instead disable THPs for the ranges, that probably can't work correctly without explicit THPs size control. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5769#note_71947
participants (4)
-
Marvin -
Paul Gofman (＠gofman) -
Vibhav Pant
-
Vibhav Pant (＠vibhavp)