[PATCH v18 0/5] MR820: Implement application hives
-- v18: kernelbase: Implement RegLoadAppKey. ntdll: Implement REG_APP_HIVE flag https://gitlab.winehq.org/wine/wine/-/merge_requests/820
From: Santino Mazza <smazza(a)codeweavers.com> Pass filename instead of openning a file handle and passing it to wineserver. This will help to simplify future implementations for load_key where we have to keep track of the file to lock it, save it, etc. For example, with application hives. --- dlls/ntdll/unix/registry.c | 18 +++++++--------- include/wine/server_protocol.h | 5 +++-- server/protocol.def | 2 +- server/registry.c | 38 ++++++++++++++++++++-------------- server/request.h | 1 - server/trace.c | 4 ++-- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index 797e32a5bf1..a15260d6713 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -700,7 +700,6 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; - HANDLE key; data_size_t len; struct object_attributes *objattr; char *unix_name; @@ -717,30 +716,29 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f if (iostatus) FIXME("iostatus is not filled\n"); get_redirect( &new_attr, &nt_name ); - if (!(ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ))) - { - ret = open_unix_file( &key, unix_name, GENERIC_READ | SYNCHRONIZE, - &new_attr, 0, 0, FILE_OPEN, 0, NULL, 0 ); - free( unix_name ); - } + ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ); free( nt_name.Buffer ); if (ret) return ret; - if ((ret = alloc_object_attributes( attr, &objattr, &len ))) return ret; + if ((ret = alloc_object_attributes( attr, &objattr, &len ))) + { + free( unix_name ); + return ret; + } objattr->attributes |= OBJ_OPENIF | OBJ_CASE_INSENSITIVE; SERVER_START_REQ( load_registry ) { - req->file = wine_server_obj_handle( key ); wine_server_add_data( req, objattr, len ); + wine_server_add_data( req, unix_name, strlen( unix_name ) ); ret = wine_server_call( req ); if (ret == STATUS_OBJECT_NAME_EXISTS) ret = STATUS_SUCCESS; } SERVER_END_REQ; - NtClose( key ); free( objattr ); + free( unix_name ); return ret; } diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index fb3168c4a6a..080fe4bb6ec 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2360,8 +2360,9 @@ struct delete_key_value_reply struct load_registry_request { struct request_header __header; - obj_handle_t file; /* VARARG(objattr,object_attributes); */ + /* VARARG(filename,string); */ + char __pad_12[4]; }; struct load_registry_reply { @@ -6324,7 +6325,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 755 +#define SERVER_PROTOCOL_VERSION 757 /* ### protocol_version end ### */ diff --git a/server/protocol.def b/server/protocol.def index d828d41d1f7..9c2a1af35c4 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1825,8 +1825,8 @@ struct process_info /* Load a registry branch from a file */ @REQ(load_registry) - obj_handle_t file; /* file to load from */ VARARG(objattr,object_attributes); /* object attributes */ + VARARG(filename,string); /* file to load name */ @END diff --git a/server/registry.c b/server/registry.c index 96ba18a0a5a..c617e3a1681 100644 --- a/server/registry.c +++ b/server/registry.c @@ -1747,24 +1747,17 @@ static void load_keys( struct key *key, const char *filename, FILE *f, int prefi } /* load a part of the registry from a file */ -static void load_registry( struct key *key, obj_handle_t handle ) +static void load_registry( struct key *key, const char *filename ) { - struct file *file; - int fd; + FILE *f; - if (!(file = get_file_obj( current->process, handle, FILE_READ_DATA ))) return; - fd = dup( get_file_unix_fd( file ) ); - release_object( file ); - if (fd != -1) + f = fopen( filename, "r" ); + if (f) { - FILE *f = fdopen( fd, "r" ); - if (f) - { - load_keys( key, NULL, f, -1 ); - fclose( f ); - } - else file_set_error(); + load_keys( key, NULL, f, -1 ); + fclose( f ); } + else file_set_error(); } /* load one of the initial registry files */ @@ -2280,10 +2273,15 @@ DECL_HANDLER(load_registry) { struct key *key, *parent = NULL; struct unicode_str name; + const char *req_data; + data_size_t req_data_len; + char *filename; const struct security_descriptor *sd; const struct object_attributes *objattr = get_req_object_attributes( &sd, &name, NULL ); if (!objattr) return; + req_data = get_req_data_after_objattr( objattr, &req_data_len ); + if (!req_data) return; if (!thread_single_check_privilege( current, SeRestorePrivilege )) { @@ -2297,7 +2295,17 @@ DECL_HANDLER(load_registry) if ((key = create_key( parent, &name, 0, KEY_WOW64_64KEY, 0, sd ))) { - load_registry( key, req->file ); + if ((filename = malloc( req_data_len + 1 ))) + { + memcpy( filename, req_data, req_data_len ); + filename[req_data_len] = 0; + + load_registry( key, filename ); + free( filename ); + } + else + set_error( STATUS_NO_MEMORY ); + release_object( key ); } if (parent) release_object( parent ); diff --git a/server/request.h b/server/request.h index 9e943cceb3c..d14cede98be 100644 --- a/server/request.h +++ b/server/request.h @@ -1230,7 +1230,6 @@ C_ASSERT( FIELD_OFFSET(struct enum_key_value_reply, namelen) == 16 ); C_ASSERT( sizeof(struct enum_key_value_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct delete_key_value_request, hkey) == 12 ); C_ASSERT( sizeof(struct delete_key_value_request) == 16 ); -C_ASSERT( FIELD_OFFSET(struct load_registry_request, file) == 12 ); C_ASSERT( sizeof(struct load_registry_request) == 16 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, parent) == 12 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, attributes) == 16 ); diff --git a/server/trace.c b/server/trace.c index c6a324bb905..5765ab40bd8 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2406,8 +2406,8 @@ static void dump_delete_key_value_request( const struct delete_key_value_request static void dump_load_registry_request( const struct load_registry_request *req ) { - fprintf( stderr, " file=%04x", req->file ); - dump_varargs_object_attributes( ", objattr=", cur_size ); + dump_varargs_object_attributes( " objattr=", cur_size ); + dump_varargs_string( ", filename=", cur_size ); } static void dump_unload_registry_request( const struct unload_registry_request *req ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/820
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/ntdll/unix/registry.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index a15260d6713..f434c380834 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -696,7 +696,7 @@ NTSTATUS WINAPI NtLoadKey2( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *fi /****************************************************************************** * NtLoadKeyEx (NTDLL.@) */ -NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *file, ULONG flags, HANDLE trustkey, +NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTES *file_attr, ULONG flags, HANDLE trustkey, HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; @@ -704,9 +704,9 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f struct object_attributes *objattr; char *unix_name; UNICODE_STRING nt_name; - OBJECT_ATTRIBUTES new_attr = *file; + OBJECT_ATTRIBUTES new_attr = *file_attr; - TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", attr, file, flags, trustkey, event, access, roothandle, iostatus ); + TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", key_attr, file_attr, flags, trustkey, event, access, roothandle, iostatus ); if (flags) FIXME( "flags %x not handled\n", flags ); if (trustkey) FIXME("trustkey parameter not supported\n"); @@ -721,7 +721,7 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *attr, OBJECT_ATTRIBUTES *f if (ret) return ret; - if ((ret = alloc_object_attributes( attr, &objattr, &len ))) + if ((ret = alloc_object_attributes( key_attr, &objattr, &len ))) { free( unix_name ); return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/820
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/ntdll/tests/reg.c | 2 +- server/registry.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index c7d9d7e4c7f..b618d879e57 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2361,7 +2361,7 @@ static void test_NtRegLoadKeyEx(void) /* Test for roothandle parameter with REG_APP_HIVE */ RtlCreateUnicodeString(&key_pathW, L"\\REGISTRY\\A\\TestKey"); status = pNtLoadKeyEx(&key_attr, &file_attr, REG_APP_HIVE, NULL, NULL, KEY_READ, &key, NULL); - todo_wine ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); + ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); todo_wine ok(key != NULL, "key is null\n"); if (key) pNtClose(key); RtlFreeUnicodeString(&key_pathW); diff --git a/server/registry.c b/server/registry.c index c617e3a1681..f611385ade4 100644 --- a/server/registry.c +++ b/server/registry.c @@ -1846,10 +1846,13 @@ void init_registry(void) 'C','u','r','r','e','n','t','V','e','r','s','i','o','n','\\', 'P','e','r','f','l','i','b','\\', '0','0','9'}; + static const WCHAR application[] = {'A'}; + static const struct unicode_str root_name = { REGISTRY, sizeof(REGISTRY) }; static const struct unicode_str HKLM_name = { HKLM, sizeof(HKLM) }; static const struct unicode_str HKU_name = { HKU_default, sizeof(HKU_default) }; static const struct unicode_str perflib_name = { perflib, sizeof(perflib) }; + static const struct unicode_str application_name = { application, sizeof(application) }; WCHAR *current_user_path; struct unicode_str current_user_str; @@ -1927,6 +1930,11 @@ void init_registry(void) release_object( key ); } + /* create application hive */ + if (!(key = create_key_recursive( root_key, &application_name, current_time ))) + fatal_error( "could not create \\REGISTRY\\A registry key\n" ); + release_object( key ); + release_object( hklm ); release_object( hkcu ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/820
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/ntdll/tests/reg.c | 4 +- dlls/ntdll/unix/registry.c | 12 ++++-- include/wine/server_protocol.h | 8 +++- server/protocol.def | 4 ++ server/registry.c | 78 +++++++++++++++++++++++++++++----- server/request.h | 6 ++- server/trace.c | 11 ++++- 7 files changed, 102 insertions(+), 21 deletions(-) diff --git a/dlls/ntdll/tests/reg.c b/dlls/ntdll/tests/reg.c index b618d879e57..a328d8e3cda 100644 --- a/dlls/ntdll/tests/reg.c +++ b/dlls/ntdll/tests/reg.c @@ -2345,7 +2345,7 @@ static void test_NtRegLoadKeyEx(void) InitializeObjectAttributes(&file_attr, &hivefile_pathW, 0, NULL, NULL); key_attr.ObjectName = &key_pathW; status = pNtLoadKeyEx(&key_attr, &file_attr, 0, NULL, NULL, KEY_READ, &key, NULL); - todo_wine ok(status == STATUS_INVALID_PARAMETER_7 || broken(status == STATUS_INVALID_PARAMETER_6) /* win7 */, "got 0x%lx\n", status); + ok(status == STATUS_INVALID_PARAMETER_7 || broken(status == STATUS_INVALID_PARAMETER_6) /* win7 */, "got 0x%lx\n", status); if (status == STATUS_INVALID_PARAMETER_6) { win_skip("NtLoadKeyEx has a different order of parameters in this windows version\n"); @@ -2362,7 +2362,7 @@ static void test_NtRegLoadKeyEx(void) RtlCreateUnicodeString(&key_pathW, L"\\REGISTRY\\A\\TestKey"); status = pNtLoadKeyEx(&key_attr, &file_attr, REG_APP_HIVE, NULL, NULL, KEY_READ, &key, NULL); ok(status == STATUS_SUCCESS, "got 0x%lx\n", status); - todo_wine ok(key != NULL, "key is null\n"); + ok(key != NULL, "key is null\n"); if (key) pNtClose(key); RtlFreeUnicodeString(&key_pathW); diff --git a/dlls/ntdll/unix/registry.c b/dlls/ntdll/unix/registry.c index f434c380834..848e935b7da 100644 --- a/dlls/ntdll/unix/registry.c +++ b/dlls/ntdll/unix/registry.c @@ -700,6 +700,7 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE HANDLE event, ACCESS_MASK access, HANDLE *roothandle, IO_STATUS_BLOCK *iostatus ) { NTSTATUS ret; + HANDLE key; data_size_t len; struct object_attributes *objattr; char *unix_name; @@ -708,13 +709,13 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE TRACE( "(%p,%p,0x%x,%p,%p,0x%x,%p,%p)\n", key_attr, file_attr, flags, trustkey, event, access, roothandle, iostatus ); - if (flags) FIXME( "flags %x not handled\n", flags ); + if (flags & ~REG_APP_HIVE) FIXME( "flags %x not handled\n", flags ); if (trustkey) FIXME("trustkey parameter not supported\n"); if (event) FIXME("event parameter not supported\n"); - if (access) FIXME("access parameter not supported\n"); - if (roothandle) FIXME("roothandle is not filled\n"); if (iostatus) FIXME("iostatus is not filled\n"); + if (roothandle && !(flags & REG_APP_HIVE)) return STATUS_INVALID_PARAMETER_7; + get_redirect( &new_attr, &nt_name ); ret = nt_to_unix_file_name( &new_attr, &unix_name, FILE_OPEN ); free( nt_name.Buffer ); @@ -730,13 +731,18 @@ NTSTATUS WINAPI NtLoadKeyEx( const OBJECT_ATTRIBUTES *key_attr, OBJECT_ATTRIBUTE SERVER_START_REQ( load_registry ) { + req->flags = flags; + req->access = access; wine_server_add_data( req, objattr, len ); wine_server_add_data( req, unix_name, strlen( unix_name ) ); ret = wine_server_call( req ); + key = wine_server_ptr_handle( reply->hkey ); if (ret == STATUS_OBJECT_NAME_EXISTS) ret = STATUS_SUCCESS; } SERVER_END_REQ; + if (roothandle) *roothandle = key; + else NtClose( key ); free( objattr ); free( unix_name ); return ret; diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 080fe4bb6ec..36fdd241499 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -2360,13 +2360,17 @@ struct delete_key_value_reply struct load_registry_request { struct request_header __header; + unsigned int flags; + unsigned int access; /* VARARG(objattr,object_attributes); */ /* VARARG(filename,string); */ - char __pad_12[4]; + char __pad_20[4]; }; struct load_registry_reply { struct reply_header __header; + obj_handle_t hkey; + char __pad_12[4]; }; @@ -6325,7 +6329,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 757 +#define SERVER_PROTOCOL_VERSION 758 /* ### protocol_version end ### */ diff --git a/server/protocol.def b/server/protocol.def index 9c2a1af35c4..4e0c4b905db 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1825,8 +1825,12 @@ struct process_info /* Load a registry branch from a file */ @REQ(load_registry) + unsigned int flags; /* flags */ + unsigned int access; /* wanted access rights */ VARARG(objattr,object_attributes); /* object attributes */ VARARG(filename,string); /* file to load name */ +(a)REPLY + obj_handle_t hkey; /* handle to root key */ @END diff --git a/server/registry.c b/server/registry.c index f611385ade4..0ada6db7f25 100644 --- a/server/registry.c +++ b/server/registry.c @@ -90,6 +90,7 @@ struct key unsigned int flags; /* flags */ timeout_t modif; /* last modification time */ struct list notify_list; /* list of notifications */ + char *filename; /* loaded file name */ }; /* key flags */ @@ -637,15 +638,6 @@ static void key_unlink_name( struct object *obj, struct object_name *name ) } } -/* close the notification associated with a handle */ -static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) -{ - struct key * key = (struct key *) obj; - struct notify *notify = find_notify( key, process, handle ); - if (notify) do_notification( key, notify, 1 ); - return 1; /* ok to close */ -} - static void key_destroy( struct object *obj ) { int i; @@ -699,6 +691,7 @@ static struct key *create_key_object( struct object *parent, const struct unicod key->last_value = -1; key->values = NULL; key->modif = modif; + key->filename = NULL; list_init( &key->notify_list ); if (options & REG_OPTION_CREATE_LINK) key->flags |= KEY_SYMLINK; @@ -1760,6 +1753,42 @@ static void load_registry( struct key *key, const char *filename ) else file_set_error(); } +static obj_handle_t load_app_registry( struct key *key, const char *filename, unsigned int access, unsigned int attributes ) +{ + static const WCHAR applicationhive_fullpath[] = {'\\', 'R', 'E', 'G', 'I', 'S', 'T', 'R', 'Y', '\\', 'A', '\\'}; + WCHAR *key_fullpath; + obj_handle_t handle = 0; + data_size_t key_fullpath_size; + + /* check if we are loading in \REGISTRY\A */ + key_fullpath = key_get_full_name( &key->obj, &key_fullpath_size ); + if (key_fullpath_size < sizeof(applicationhive_fullpath) || + memcmp( key_fullpath, applicationhive_fullpath, sizeof(applicationhive_fullpath) )) + { + set_error( STATUS_PRIVILEGE_NOT_HELD ); + } + free( key_fullpath ); + if (get_error()) return 0; + + load_registry( key, filename ); + if (get_error()) + { + delete_key( key, 1 ); + return 0; + } + + key->filename = strdup( filename ); + if (!key->filename) + { + delete_key( key, 1 ); + set_error( STATUS_NO_MEMORY ); + return 0; + } + + handle = alloc_handle( current->process, key, access, attributes ); + return handle; +} + /* load one of the initial registry files */ static int load_init_registry_from_file( const char *filename, struct key *key ) { @@ -2120,6 +2149,29 @@ void flush_registry(void) if (fchdir( server_dir_fd ) == -1) fatal_error( "chdir to server dir: %s\n", strerror( errno )); } +/* close the notification associated with a handle */ +static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) +{ + struct key *key = (struct key *) obj; + FILE *file; + struct notify *notify = find_notify( key, process, handle ); + if (notify) do_notification( key, notify, 1 ); + if (key->filename) + { + file = fopen( key->filename, "w" ); + if (file) + { + save_all_subkeys( key, file ); + if (fclose( file )) file_set_error(); + } + else + file_set_error(); + delete_key( key, 1 ); + free( key->filename ); + } + return 1; /* ok to close */ +} + /* determine if the thread is wow64 (32-bit client running on 64-bit prefix) */ static int is_wow64_thread( struct thread *thread ) { @@ -2291,7 +2343,7 @@ DECL_HANDLER(load_registry) req_data = get_req_data_after_objattr( objattr, &req_data_len ); if (!req_data) return; - if (!thread_single_check_privilege( current, SeRestorePrivilege )) + if (!(req->flags & REG_APP_HIVE) && !thread_single_check_privilege( current, SeRestorePrivilege )) { set_error( STATUS_PRIVILEGE_NOT_HELD ); return; @@ -2308,7 +2360,11 @@ DECL_HANDLER(load_registry) memcpy( filename, req_data, req_data_len ); filename[req_data_len] = 0; - load_registry( key, filename ); + if (req->flags & REG_APP_HIVE) + reply->hkey = load_app_registry( key, filename, req->access, objattr->attributes ); + else + load_registry( key, filename ); + free( filename ); } else diff --git a/server/request.h b/server/request.h index d14cede98be..faf66287b60 100644 --- a/server/request.h +++ b/server/request.h @@ -1230,7 +1230,11 @@ C_ASSERT( FIELD_OFFSET(struct enum_key_value_reply, namelen) == 16 ); C_ASSERT( sizeof(struct enum_key_value_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct delete_key_value_request, hkey) == 12 ); C_ASSERT( sizeof(struct delete_key_value_request) == 16 ); -C_ASSERT( sizeof(struct load_registry_request) == 16 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_request, flags) == 12 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_request, access) == 16 ); +C_ASSERT( sizeof(struct load_registry_request) == 24 ); +C_ASSERT( FIELD_OFFSET(struct load_registry_reply, hkey) == 8 ); +C_ASSERT( sizeof(struct load_registry_reply) == 16 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, parent) == 12 ); C_ASSERT( FIELD_OFFSET(struct unload_registry_request, attributes) == 16 ); C_ASSERT( sizeof(struct unload_registry_request) == 24 ); diff --git a/server/trace.c b/server/trace.c index 5765ab40bd8..97435dc7493 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2406,10 +2406,17 @@ static void dump_delete_key_value_request( const struct delete_key_value_request static void dump_load_registry_request( const struct load_registry_request *req ) { - dump_varargs_object_attributes( " objattr=", cur_size ); + fprintf( stderr, " flags=%08x", req->flags ); + fprintf( stderr, ", access=%08x", req->access ); + dump_varargs_object_attributes( ", objattr=", cur_size ); dump_varargs_string( ", filename=", cur_size ); } +static void dump_load_registry_reply( const struct load_registry_reply *req ) +{ + fprintf( stderr, " hkey=%04x", req->hkey ); +} + static void dump_unload_registry_request( const struct unload_registry_request *req ) { fprintf( stderr, " parent=%04x", req->parent ); @@ -4869,7 +4876,7 @@ static const dump_func reply_dumpers[REQ_NB_REQUESTS] = { (dump_func)dump_get_key_value_reply, (dump_func)dump_enum_key_value_reply, NULL, - NULL, + (dump_func)dump_load_registry_reply, NULL, NULL, NULL, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/820
From: Santino Mazza <smazza(a)codeweavers.com> --- dlls/advapi32/tests/registry.c | 6 +-- dlls/kernelbase/registry.c | 71 ++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/dlls/advapi32/tests/registry.c b/dlls/advapi32/tests/registry.c index 55a8074f1b7..e2024ec8899 100644 --- a/dlls/advapi32/tests/registry.c +++ b/dlls/advapi32/tests/registry.c @@ -1652,7 +1652,7 @@ static void test_reg_load_app_key(void) ok(appkey != NULL, "got a null key\n"); ret = RegSetValueExA(appkey, "testkey", 0, REG_BINARY, test_data, sizeof(test_data)); - todo_wine ok(ret == ERROR_SUCCESS, "couldn't set key value %lx\n", ret); + ok(ret == ERROR_SUCCESS, "couldn't set key value %lx\n", ret); RegCloseKey(appkey); wait_file_available(hivefilepath); @@ -1665,9 +1665,9 @@ static void test_reg_load_app_key(void) size = sizeof(test_data); memset(output, 0xff, sizeof(output)); ret = RegGetValueA(appkey, NULL, "testkey", RRF_RT_REG_BINARY, NULL, output, &size); - todo_wine ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); + ok(ret == ERROR_SUCCESS, "expected ERROR_SUCCESS, got %ld\n", ret); ok(size == sizeof(test_data), "size doesn't match %ld != %ld\n", size, (DWORD)sizeof(test_data)); - todo_wine ok(!memcmp(test_data, output, sizeof(test_data)), "output is not what expected\n"); + ok(!memcmp(test_data, output, sizeof(test_data)), "output is not what expected\n"); RegCloseKey(appkey); diff --git a/dlls/kernelbase/registry.c b/dlls/kernelbase/registry.c index 91462d80e06..ac4dd97776a 100644 --- a/dlls/kernelbase/registry.c +++ b/dlls/kernelbase/registry.c @@ -3080,35 +3080,82 @@ cleanup: return ret; } +static void generate_string_uuid(WCHAR *out, DWORD out_size) +{ + UUID uuid; + ULONG *ptr = (ULONG*)&uuid; + LARGE_INTEGER ft; + ULONG seed; + int i; + + NtQuerySystemTime(&ft); + seed = ft.LowPart; + for (i = 0; i < sizeof(UUID)/sizeof(ULONG); i++, ptr++) + *ptr = RtlUniform(&seed); + + uuid.Data3 &= 0x0fff; + uuid.Data3 |= (4 << 12); + uuid.Data4[0] &= 0x3f; + uuid.Data4[0] |= 0x80; + + swprintf(out, out_size, L"{%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X}", uuid.Data1, + uuid.Data2, uuid.Data3, uuid.Data4[0], uuid.Data4[1], uuid.Data4[2], uuid.Data4[3], + uuid.Data4[4], uuid.Data4[5], uuid.Data4[6], uuid.Data4[7]); +} /****************************************************************************** * RegLoadAppKeyA (kernelbase.@) * */ -LSTATUS WINAPI RegLoadAppKeyA(const char *file, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) +LSTATUS WINAPI RegLoadAppKeyA(const char *filename, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) { - FIXME("%s %p %lu %lu %lu: stub\n", wine_dbgstr_a(file), result, sam, options, reserved); - - if (!file || reserved) - return ERROR_INVALID_PARAMETER; + UNICODE_STRING filenameW; + LSTATUS status; + TRACE("%s %p %lu %lu %lu\n", wine_dbgstr_a(filename), result, sam, options, reserved); - *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + RtlCreateUnicodeStringFromAsciiz(&filenameW, filename); + status = RegLoadAppKeyW(filenameW.Buffer, result, sam, options, reserved); + RtlFreeUnicodeString(&filenameW); + return status; } /****************************************************************************** * RegLoadAppKeyW (kernelbase.@) * */ -LSTATUS WINAPI RegLoadAppKeyW(const WCHAR *file, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) +LSTATUS WINAPI RegLoadAppKeyW(const WCHAR *filename, HKEY *result, REGSAM sam, DWORD options, DWORD reserved) { - FIXME("%s %p %lu %lu %lu: stub\n", wine_dbgstr_w(file), result, sam, options, reserved); + NTSTATUS status; + WCHAR application_root[13] = L"\\REGISTRY\\A\\"; + WCHAR rootguid_str[39]; + WCHAR *destkey_path_tmp; + UNICODE_STRING destkey_path, filenameW; + OBJECT_ATTRIBUTES destkey, file; - if (!file || reserved) + TRACE("%s %p %lu %lu %lu\n", wine_dbgstr_w(filename), result, sam, options, reserved); + + if (!filename || reserved) return ERROR_INVALID_PARAMETER; - *result = (HKEY)0xdeadbeef; - return ERROR_SUCCESS; + InitializeObjectAttributes(&destkey, &destkey_path, 0, 0, 0); + RtlDosPathNameToNtPathName_U(filename, &filenameW, NULL, NULL); + InitializeObjectAttributes(&file, &filenameW, 0, 0, 0); + + generate_string_uuid(rootguid_str, sizeof(rootguid_str)); + + destkey_path_tmp = heap_alloc_zero(sizeof(application_root) + sizeof(rootguid_str)); + wcscat(destkey_path_tmp, application_root); + wcscat(destkey_path_tmp, rootguid_str); + + RtlCreateUnicodeString(&destkey_path, destkey_path_tmp); + heap_free(destkey_path_tmp); + + status = NtLoadKeyEx(&destkey, &file, REG_APP_HIVE, 0, 0, sam, (HANDLE *)result, 0); + + RtlFreeUnicodeString(&destkey_path); + RtlFreeUnicodeString(&filenameW); + + return RtlNtStatusToDosError(status); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/820
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/820
Alexandre Julliard (@julliard) commented about server/registry.c:
+} + +static obj_handle_t load_app_registry( struct key *key, const char *filename, unsigned int access, unsigned int attributes ) +{ + static const WCHAR applicationhive_fullpath[] = {'\\', 'R', 'E', 'G', 'I', 'S', 'T', 'R', 'Y', '\\', 'A', '\\'}; + WCHAR *key_fullpath; + obj_handle_t handle = 0; + data_size_t key_fullpath_size; + + /* check if we are loading in \REGISTRY\A */ + key_fullpath = key_get_full_name( &key->obj, &key_fullpath_size ); + if (key_fullpath_size < sizeof(applicationhive_fullpath) || + memcmp( key_fullpath, applicationhive_fullpath, sizeof(applicationhive_fullpath) )) + { + set_error( STATUS_PRIVILEGE_NOT_HELD ); + } You should store a pointer to the A root key and compare with the parent pointer instead of comparing names.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9746
Alexandre Julliard (@julliard) commented about server/registry.c:
+ if ((filename = malloc( req_data_len + 1 ))) + { + memcpy( filename, req_data, req_data_len ); + filename[req_data_len] = 0; + + if (req->flags & REG_APP_HIVE) + reply->hkey = load_app_registry( key, filename, req->access, objattr->attributes ); + else + load_registry( key, filename ); + + free( filename ); + } + else + set_error( STATUS_NO_MEMORY ); + release_object( key ); I suspect that it would be cleaner to have a separate request for loading app keys.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9747
Alexandre Julliard (@julliard) commented about server/registry.c:
+static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) +{ + struct key *key = (struct key *) obj; + FILE *file; + struct notify *notify = find_notify( key, process, handle ); + if (notify) do_notification( key, notify, 1 ); + if (key->filename) + { + file = fopen( key->filename, "w" ); + if (file) + { + save_all_subkeys( key, file ); + if (fclose( file )) file_set_error(); + } + else + file_set_error(); Setting the error in a close notification is not useful.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9748
Alexandre Julliard (@julliard) commented about server/registry.c:
if (fchdir( server_dir_fd ) == -1) fatal_error( "chdir to server dir: %s\n", strerror( errno )); }
+/* close the notification associated with a handle */ +static int key_close_handle( struct object *obj, struct process *process, obj_handle_t handle ) +{ + struct key *key = (struct key *) obj; + FILE *file; + struct notify *notify = find_notify( key, process, handle ); + if (notify) do_notification( key, notify, 1 ); + if (key->filename) + { + file = fopen( key->filename, "w" );
What happens if the handle was duplicated? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9749
On Thu Sep 29 13:49:49 2022 +0000, Alexandre Julliard wrote:
I suspect that it would be cleaner to have a separate request for loading app keys. I'm not sure, we will have duplicated code inside NtLoadKeyEx, for making both requests depending on the flags.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9804
On Thu Sep 29 13:49:50 2022 +0000, Alexandre Julliard wrote:
What happens if the handle was duplicated? mmm, didn't think of this, I think it depends on the flag REG_PROCESS_PRIVATE, if the flag is set the handle shouldn't be able to be duplicated, so when it's not I think I'll have to check if other process has access to the key somehow before deleting the key when key_close_handle is called.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9805
On Thu Sep 29 13:49:49 2022 +0000, Alexandre Julliard wrote:
You should store a pointer to the A root key and compare with the parent pointer instead of comparing names. You mean leave opened the app registry object inside a global variable or something? In RegLoadAppKey I pass an absolute path to \REGISTRY\A, I don't set the root attribute. Does the object still receives the parent attribute set to \REGISTRY\A even without the root attribute set?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9807
In RegLoadAppKey I pass an absolute path to \\REGISTRY\\A, I don't set the root attribute. Does the object still receives the parent attribute set to \\REGISTRY\\A even without the root attribute set?
key->obj.parent will point to the parent. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9820
On Thu Sep 29 23:37:31 2022 +0000, Santino Mazza wrote:
mmm, didn't think of this, I think it depends on the flag REG_PROCESS_PRIVATE, if the flag is set the handle shouldn't be able to be duplicated, so when it's not I think I'll have to check if other process has access to the key somehow before deleting the key when key_close_handle is called. While the cross-process case is interesting, the more pressing point is simply duplicating in the current process. Probably this code should be in the `key_destroy()` handler, i.e. the hive would get saved when the last handle is closed.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/820#note_9821
participants (4)
-
Alexandre Julliard (＠julliard) -
Huw Davies (＠huw)
-
Santino Mazza
-
Santino Mazza (＠tati1454)