[PATCH 0/3] MR9926: ntdll: Make RtlEncodePointer / RtlDecodePointer compatible.
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/ntdll/unix/system.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index 350bcea4f86..08774d79de7 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -3024,6 +3024,22 @@ static void read_dev_urandom( void *buf, ULONG len ) else WARN( "can't open /dev/urandom\n" ); } +static void get_random( void *buf, ULONG len ) +{ +#ifdef HAVE_GETRANDOM + int ret; + do + { + ret = getrandom( buf, len, 0 ); + } + while (ret == -1 && errno == EINTR); + + if (ret == -1 && errno == ENOSYS) read_dev_urandom( buf, len ); +#else + read_dev_urandom( buf, len ); +#endif +} + static unsigned int get_system_process_info( SYSTEM_INFORMATION_CLASS class, void *info, ULONG size, ULONG *len ) { unsigned int process_count, total_thread_count, total_name_len, i, j; @@ -3488,21 +3504,7 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class, if (size >= len) { if (!info) ret = STATUS_ACCESS_VIOLATION; - else - { -#ifdef HAVE_GETRANDOM - int ret; - do - { - ret = getrandom( info, len, 0 ); - } - while (ret == -1 && errno == EINTR); - - if (ret == -1 && errno == ENOSYS) read_dev_urandom( info, len ); -#else - read_dev_urandom( info, len ); -#endif - } + else get_random( info, len ); } else ret = STATUS_INFO_LENGTH_MISMATCH; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9926
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/kernel32/tests/process.c | 4 ++++ dlls/ntdll/unix/loader.c | 2 ++ dlls/ntdll/unix/process.c | 4 ++-- dlls/ntdll/unix/system.c | 2 +- dlls/ntdll/unix/unix_private.h | 2 ++ 5 files changed, 11 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index fd7f5a8d34c..f67189205d5 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -4369,8 +4369,12 @@ static void test_process_info(HANDLE hproc) break; case ProcessCookie: if (is_current) + { ok(status == STATUS_SUCCESS || status == STATUS_INVALID_PARAMETER /* before win8 */, "for info %lu got %08lx (ret_len %lu)\n", i, status, ret_len); + if (!status) + ok( *(ULONG *)buf, "got 0.\n" ); + } else ok(status == STATUS_INVALID_PARAMETER /* before win8 */ || status == STATUS_ACCESS_DENIED, "for info %lu got %08lx (ret_len %lu)\n", i, status, ret_len); diff --git a/dlls/ntdll/unix/loader.c b/dlls/ntdll/unix/loader.c index cb700dd80e8..c1ac262e5d6 100644 --- a/dlls/ntdll/unix/loader.c +++ b/dlls/ntdll/unix/loader.c @@ -1914,6 +1914,8 @@ static void start_main_thread(void) startup_info_size = server_init_process(); virtual_map_user_shared_data(); init_cpu_info(); + process_cookie = 0xdeadbeef; + get_random( &process_cookie, sizeof(process_cookie) ); init_files(); init_startup_info(); *(ULONG_PTR *)&peb->CloudFileFlags = get_image_address(); diff --git a/dlls/ntdll/unix/process.c b/dlls/ntdll/unix/process.c index 4c2e7eba49a..15405f3d0f6 100644 --- a/dlls/ntdll/unix/process.c +++ b/dlls/ntdll/unix/process.c @@ -77,6 +77,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(process); static ULONG execute_flags = MEM_EXECUTE_OPTION_DISABLE; static UINT process_error_mode; +ULONG process_cookie; static char **build_argv( const UNICODE_STRING *cmdline, int reserved ) { @@ -1506,11 +1507,10 @@ NTSTATUS WINAPI NtQueryInformationProcess( HANDLE handle, PROCESSINFOCLASS class break; case ProcessCookie: - FIXME( "ProcessCookie (%p,%p,0x%08x,%p) stub\n", handle, info, size, ret_len ); if (handle == NtCurrentProcess()) { len = sizeof(ULONG); - if (size == len) *(ULONG *)info = 0; + if (size == len) *(ULONG *)info = process_cookie; else ret = STATUS_INFO_LENGTH_MISMATCH; } else ret = STATUS_INVALID_PARAMETER; diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index 08774d79de7..31df6fcf496 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -3024,7 +3024,7 @@ static void read_dev_urandom( void *buf, ULONG len ) else WARN( "can't open /dev/urandom\n" ); } -static void get_random( void *buf, ULONG len ) +void get_random( void *buf, ULONG len ) { #ifdef HAVE_GETRANDOM int ret; diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 3932b175a96..727a443b0d9 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -203,6 +203,7 @@ extern timeout_t server_start_time; extern sigset_t server_block_set; extern pthread_mutex_t fd_cache_mutex; extern struct _KUSER_SHARED_DATA *user_shared_data; +extern ULONG process_cookie; extern void init_environment(void); extern void init_startup_info(void); @@ -271,6 +272,7 @@ extern NTSTATUS get_thread_context( HANDLE handle, void *context, BOOL *self, US extern unsigned int alloc_object_attributes( const OBJECT_ATTRIBUTES *attr, struct object_attributes **ret, data_size_t *ret_len ); extern NTSTATUS system_time_precise( void *args ); +extern void get_random( void *buf, ULONG len ); extern void *anon_mmap_fixed( void *start, size_t size, int prot, int flags ); extern void *anon_mmap_alloc( size_t size, int prot ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9926
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/ntdll/rtl.c | 45 ++++++++++++++--------------- dlls/ntdll/tests/rtl.c | 64 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 22 deletions(-) diff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c index b63829aff12..84ca9f1b4e5 100644 --- a/dlls/ntdll/rtl.c +++ b/dlls/ntdll/rtl.c @@ -1022,45 +1022,46 @@ ULONG WINAPI RtlRandomEx( ULONG *seed ) } /*********************************************************************** - * get_pointer_obfuscator (internal) + * get_process_cookie (internal) */ -static DWORD_PTR get_pointer_obfuscator( void ) +static ULONG get_process_cookie(void) { - static DWORD_PTR pointer_obfuscator; + static ULONG process_cookie; - if (!pointer_obfuscator) + if (!process_cookie) { - ULONG seed = NtGetTickCount(); - ULONG_PTR rand; + ULONG cookie; - /* generate a random value for the obfuscator */ - rand = RtlUniform( &seed ); - - /* handle 64bit pointers */ - rand ^= (ULONG_PTR)RtlUniform( &seed ) << ((sizeof (DWORD_PTR) - sizeof (ULONG))*8); - - /* set the high bits so dereferencing obfuscated pointers will (usually) crash */ - rand |= (ULONG_PTR)0xc0000000 << ((sizeof (DWORD_PTR) - sizeof (ULONG))*8); - - InterlockedCompareExchangePointer( (void**) &pointer_obfuscator, (void*) rand, NULL ); + NtQueryInformationProcess( GetCurrentProcess(), ProcessCookie, &cookie, sizeof(cookie), NULL ); + InterlockedCompareExchange( (LONG volatile *)&process_cookie, cookie, 0 ); } - - return pointer_obfuscator; + return process_cookie; } +#define BIT_COUNT_IN_POINTER (sizeof(void *) * 8) + /************************************************************************* * RtlEncodePointer [NTDLL.@] */ PVOID WINAPI RtlEncodePointer( PVOID ptr ) { - DWORD_PTR ptrval = (DWORD_PTR) ptr; - return (PVOID)(ptrval ^ get_pointer_obfuscator()); + ULONG cookie = get_process_cookie(); + ULONG rotate = cookie % BIT_COUNT_IN_POINTER; + ULONG_PTR ptrval = (ULONG_PTR)ptr ^ cookie; + + return (void *)((ptrval >> rotate) | (ptrval << (BIT_COUNT_IN_POINTER - rotate))); } +/************************************************************************* + * RtlDecodePointer [NTDLL.@] + */ PVOID WINAPI RtlDecodePointer( PVOID ptr ) { - DWORD_PTR ptrval = (DWORD_PTR) ptr; - return (PVOID)(ptrval ^ get_pointer_obfuscator()); + ULONG cookie = get_process_cookie(); + ULONG rotate = cookie % BIT_COUNT_IN_POINTER; + ULONG_PTR ptrval = (ULONG_PTR)ptr; + + return (void *)(((ptrval << rotate) | (ptrval >> (BIT_COUNT_IN_POINTER - rotate))) ^ cookie); } /****************************************************************************** diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c index f5db5cdc6e2..d0b5310fe42 100644 --- a/dlls/ntdll/tests/rtl.c +++ b/dlls/ntdll/tests/rtl.c @@ -5476,6 +5476,69 @@ static void test_RtlDeriveCapabilitySidsFromName(void) free( group_sid ); } +static ULONG_PTR rotate_bits_right( ULONG_PTR v, ULONG count ) +{ + static const unsigned int bits = sizeof(v) * 8; + + count %= bits; + return (v >> count) | (v << (bits - count)); +} + +static ULONG_PTR rotate_bits_left( ULONG_PTR v, ULONG count ) +{ + static const unsigned int bits = sizeof(v) * 8; + + count %= bits; + return (v << count) | (v >> (bits - count)); +} + +static ULONG process_cookie; + +static void *encode_pointer( void *ptr ) +{ + DWORD_PTR ptrval = (DWORD_PTR)ptr; + return (void *)rotate_bits_right( ptrval ^ process_cookie, process_cookie ); +} + +static void *decode_pointer( void *ptr ) +{ + DWORD_PTR ptrval = (DWORD_PTR)ptr; + return (void *)(rotate_bits_left( ptrval, process_cookie ) ^ process_cookie ); +} + +static void test_pointer_encoding(void) +{ + void *v, *expected; + + if (NtQueryInformationProcess( GetCurrentProcess(), ProcessCookie, &process_cookie, sizeof(process_cookie), NULL )) + { + win_skip( "Could not get process cookie, skipping tests.\n" ); + return; + } + ok( process_cookie, "got 0.\n" ); + + v = RtlEncodePointer( NULL ); + expected = encode_pointer( NULL ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); + v = RtlDecodePointer( v ); + expected = decode_pointer( expected ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); + + v = RtlEncodePointer( (void *)(ULONG_PTR)1 ); + expected = encode_pointer( (void *)(ULONG_PTR)1 ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); + v = RtlDecodePointer( v ); + expected = decode_pointer( expected ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); + + v = RtlEncodePointer( (void *)(ULONG_PTR)0xdeadbeeffeedcafe ); + expected = encode_pointer( (void *)(ULONG_PTR)0xdeadbeeffeedcafe ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); + v = RtlDecodePointer( v ); + expected = decode_pointer( expected ); + ok( v == expected, "got %p, expected %p.\n", v, expected ); +} + START_TEST(rtl) { InitFunctionPtrs(); @@ -5546,4 +5609,5 @@ START_TEST(rtl) test_RtlGetElementGenericTable(); test_RtlCreateServiceSid(); test_RtlDeriveCapabilitySidsFromName(); + test_pointer_encoding(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9926
Alfred Agrell (@Alcaro) commented about dlls/ntdll/tests/rtl.c:
free( group_sid ); }
+static ULONG_PTR rotate_bits_right( ULONG_PTR v, ULONG count ) +{ + static const unsigned int bits = sizeof(v) * 8; + + count %= bits; + return (v >> count) | (v << (bits - count));
If count==0, bits-count = 64, and v<<64 is UB. (Or 32 on 32bit builds, but still UB.) Consider instead `return (v >> (count % bits)) | (v << ((bits - count) % bits));`. https://godbolt.org/z/9nWP6zK8P (Or stdc_rotate_right, except that one doesn't exist until C23.) Same for the rotate left function below, and the actual encode/decode pointer functions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9926#note_127365
Alfred Agrell (@Alcaro) commented about dlls/kernel32/tests/process.c:
break; case ProcessCookie: if (is_current) + { ok(status == STATUS_SUCCESS || status == STATUS_INVALID_PARAMETER /* before win8 */, "for info %lu got %08lx (ret_len %lu)\n", i, status, ret_len); + if (!status) + ok( *(ULONG *)buf, "got 0.\n" );
Took me a few attempts to read this line correctly. I'd add a !=0. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9926#note_127366
participants (3)
-
Alfred Agrell (＠Alcaro) -
Paul Gofman
-
Paul Gofman (＠gofman)