From f278e30d210e7e7173baae796786b34cf89f5694 Mon Sep 17 00:00:00 2001 From: Silent Date: Sun, 1 Sep 2019 16:43:27 +0200 Subject: [PATCH 1/3] [Kernel] Fix SHA1 and SHA256 state endianness Fixes malfunctioning SHA256 hash, corrupting its state between Update and Finish calls. --- src/xenia/kernel/xboxkrnl/xboxkrnl_crypt.cc | 53 +++++++++++---------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_crypt.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_crypt.cc index abfab381e..9a8e4a001 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_crypt.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_crypt.cc @@ -88,20 +88,16 @@ static_assert_size(XECRYPT_SHA_STATE, 0x58); void InitSha1(sha1::SHA1* sha, const XECRYPT_SHA_STATE* state) { uint32_t digest[5]; - for (int i = 0; i < 5; i++) { - digest[i] = state->state[i]; - } + std::copy(std::begin(state->state), std::end(state->state), digest); sha->init(digest, state->buffer, state->count); } -void StoreSha1(sha1::SHA1* sha, XECRYPT_SHA_STATE* state) { - for (int i = 0; i < 5; i++) { - state->state[i] = sha->getDigest()[i]; - } +void StoreSha1(const sha1::SHA1* sha, XECRYPT_SHA_STATE* state) { + std::copy_n(sha->getDigest(), xe::countof(state->state), state->state); state->count = static_cast(sha->getByteCount()); - std::memcpy(state->buffer, sha->getBlock(), sha->getBlockByteIndex()); + std::copy_n(sha->getBlock(), sha->getBlockByteIndex(), state->buffer); } void XeCryptShaInit(pointer_t sha_state) { @@ -127,15 +123,16 @@ void XeCryptShaUpdate(pointer_t sha_state, lpvoid_t input, DECLARE_XBOXKRNL_EXPORT1(XeCryptShaUpdate, kNone, kImplemented); void XeCryptShaFinal(pointer_t sha_state, - pointer_t> out, dword_t out_size) { + pointer_t out, dword_t out_size) { sha1::SHA1 sha; InitSha1(&sha, sha_state); uint8_t digest[0x14]; sha.finalize(digest); - std::memcpy(out, digest, std::min((uint32_t)out_size, 0x14u)); - std::memcpy(sha_state->state, digest, 0x14); + std::copy_n(digest, std::min(xe::countof(digest), out_size), + static_cast(out)); + std::copy_n(sha.getDigest(), xe::countof(sha_state->state), sha_state->state); } DECLARE_XBOXKRNL_EXPORT1(XeCryptShaFinal, kNone, kImplemented); @@ -156,7 +153,8 @@ void XeCryptSha(lpvoid_t input_1, dword_t input_1_size, lpvoid_t input_2, uint8_t digest[0x14]; sha.finalize(digest); - std::memcpy(output, digest, std::min((uint32_t)output_size, 0x14u)); + std::copy_n(digest, std::min(xe::countof(digest), output_size), + output.as()); } DECLARE_XBOXKRNL_EXPORT1(XeCryptSha, kNone, kImplemented); @@ -184,30 +182,37 @@ DECLARE_XBOXKRNL_EXPORT1(XeCryptSha256Init, kNone, kImplemented); void XeCryptSha256Update(pointer_t sha_state, lpvoid_t input, dword_t input_size) { sha256::SHA256 sha; - std::memcpy(sha.getHashValues(), sha_state->state, 8 * 4); - std::memcpy(sha.getBuffer(), sha_state->buffer, 64); + std::copy(std::begin(sha_state->state), std::end(sha_state->state), + sha.getHashValues()); + std::copy(std::begin(sha_state->buffer), std::end(sha_state->buffer), + sha.getBuffer()); sha.setTotalSize(sha_state->count); sha.add(input, input_size); - std::memcpy(sha_state->state, sha.getHashValues(), 8 * 4); - std::memcpy(sha_state->buffer, sha.getBuffer(), 64); - sha_state->count = uint32_t(sha.getTotalSize()); + std::copy_n(sha.getHashValues(), xe::countof(sha_state->state), + sha_state->state); + std::copy_n(sha.getBuffer(), xe::countof(sha_state->buffer), + sha_state->buffer); + sha_state->count = static_cast(sha.getTotalSize()); } DECLARE_XBOXKRNL_EXPORT1(XeCryptSha256Update, kNone, kImplemented); void XeCryptSha256Final(pointer_t sha_state, - pointer_t> out, dword_t out_size) { + pointer_t out, dword_t out_size) { sha256::SHA256 sha; - std::memcpy(sha.getHashValues(), sha_state->state, 8 * 4); - std::memcpy(sha.getBuffer(), sha_state->buffer, 64); + std::copy(std::begin(sha_state->state), std::end(sha_state->state), + sha.getHashValues()); + std::copy(std::begin(sha_state->buffer), std::end(sha_state->buffer), + sha.getBuffer()); sha.setTotalSize(sha_state->count); - uint32_t hash[8]; - sha.getHash(reinterpret_cast(hash)); + uint8_t hash[32]; + sha.getHash(hash); - std::memcpy(out, hash, std::min(uint32_t(out_size), 32u)); - std::memcpy(sha_state->buffer, hash, 32); + std::copy_n(hash, std::min(xe::countof(hash), out_size), + static_cast(out)); + std::copy(std::begin(hash), std::end(hash), sha_state->buffer); } DECLARE_XBOXKRNL_EXPORT1(XeCryptSha256Final, kNone, kImplemented); From 9d48e904da7e0bc4c450c4e269eea6e9c3fb6323 Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 2 Sep 2019 21:44:43 +0200 Subject: [PATCH 2/3] [Kernel] (Partially) fix module refcounting .xex module handles were retained twice in several places, possibly causing them to leak. More placed may have to be fixed too. --- src/xenia/kernel/kernel_state.cc | 8 +++----- src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc | 6 ++++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/xenia/kernel/kernel_state.cc b/src/xenia/kernel/kernel_state.cc index 57097148e..83f10bbdd 100644 --- a/src/xenia/kernel/kernel_state.cc +++ b/src/xenia/kernel/kernel_state.cc @@ -367,8 +367,7 @@ object_ref KernelState::LoadUserModule(const char* raw_name, // See if we've already loaded it for (auto& existing_module : user_modules_) { if (existing_module->path() == path) { - existing_module->Retain(); - return retain_object(existing_module.get()); + return existing_module; } } @@ -378,14 +377,13 @@ object_ref KernelState::LoadUserModule(const char* raw_name, module = object_ref(new UserModule(this)); X_STATUS status = module->LoadFromFile(path); if (XFAILED(status)) { - object_table()->RemoveHandle(module->handle()); + object_table()->ReleaseHandle(module->handle()); return nullptr; } global_lock.lock(); - // Retain when putting into the listing. - module->Retain(); + // Putting into the listing automatically retains. user_modules_.push_back(module); } diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc index 81a44c553..f6d93a9f2 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc @@ -96,8 +96,10 @@ dword_result_t XexLoadImage(lpstring_t module_name, dword_t module_flags, // Not found; attempt to load as a user module. auto user_module = kernel_state()->LoadUserModule(module_name); if (user_module) { - user_module->Retain(); - hmodule = user_module->hmodule_ptr(); + // Give up object ownership, this reference will be released by the last + // XexUnloadImage call + auto user_module_raw = user_module.release(); + hmodule = user_module_raw->hmodule_ptr(); result = X_STATUS_SUCCESS; } } From 38bf6c88221fa724efe2accea3d61033a7ccab37 Mon Sep 17 00:00:00 2001 From: Silent Date: Mon, 2 Sep 2019 21:48:51 +0200 Subject: [PATCH 3/3] [Kernel] Make XexUnloadImage fully release the image Previously XexUnloadImage did not cleanup the image fully, and if XexLoadImage was to be called again on the same module, it was not initialized fully, leading to a crash when using it. --- src/xenia/kernel/kernel_state.cc | 33 +++++++++++++++++++ src/xenia/kernel/kernel_state.h | 2 ++ src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc | 3 +- 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/xenia/kernel/kernel_state.cc b/src/xenia/kernel/kernel_state.cc index 83f10bbdd..9366c1160 100644 --- a/src/xenia/kernel/kernel_state.cc +++ b/src/xenia/kernel/kernel_state.cc @@ -405,6 +405,39 @@ object_ref KernelState::LoadUserModule(const char* raw_name, return module; } +void KernelState::UnloadUserModule(const object_ref& module, + bool call_entry) { + auto global_lock = global_critical_region_.Acquire(); + + if (module->is_dll_module() && module->entry_point() && call_entry) { + // Call DllMain(DLL_PROCESS_DETACH): + // https://msdn.microsoft.com/en-us/library/windows/desktop/ms682583%28v=vs.85%29.aspx + uint64_t args[] = { + module->handle(), + 0, // DLL_PROCESS_DETACH + 0, // 0 for now, assume XexUnloadImage is like FreeLibrary + }; + auto thread_state = XThread::GetCurrentThread()->thread_state(); + processor()->Execute(thread_state, module->entry_point(), args, + xe::countof(args)); + } + + auto iter = std::find_if( + user_modules_.begin(), user_modules_.end(), + [&module](const auto& e) { return e->path() == module->path(); }); + assert_true(iter != user_modules_.end()); // Unloading an unregistered module + // is probably really bad + user_modules_.erase(iter); + + // Ensure this module was not somehow registered twice + assert_true(std::find_if(user_modules_.begin(), user_modules_.end(), + [&module](const auto& e) { + return e->path() == module->path(); + }) == user_modules_.end()); + + object_table()->ReleaseHandle(module->handle()); +} + void KernelState::TerminateTitle() { XELOGD("KernelState::TerminateTitle"); auto global_lock = global_critical_region_.Acquire(); diff --git a/src/xenia/kernel/kernel_state.h b/src/xenia/kernel/kernel_state.h index 19fbc5825..95ce08fb9 100644 --- a/src/xenia/kernel/kernel_state.h +++ b/src/xenia/kernel/kernel_state.h @@ -131,6 +131,8 @@ class KernelState { void SetExecutableModule(object_ref module); object_ref LoadUserModule(const char* name, bool call_entry = true); + void UnloadUserModule(const object_ref& module, + bool call_entry = true); object_ref GetKernelModule(const char* name); template diff --git a/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc b/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc index f6d93a9f2..5c5ed2177 100644 --- a/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc +++ b/src/xenia/kernel/xboxkrnl/xboxkrnl_modules.cc @@ -129,7 +129,8 @@ dword_result_t XexUnloadImage(lpvoid_t hmodule) { if (--ldr_data->load_count == 0) { // No more references, free it. module->Release(); - kernel_state()->object_table()->RemoveHandle(module->handle()); + kernel_state()->UnloadUserModule(object_ref( + reinterpret_cast(module.release()))); } }