From c9c1ba0952c902ceca00717ab49d0a3f21feaa8e Mon Sep 17 00:00:00 2001 From: Subv Date: Sun, 5 Nov 2017 12:50:22 -0500 Subject: [PATCH 1/2] Kernel/IPC: Implement StaticBuffer translation for HLE services that use the HLERequestContext architecture. The real kernel requires services to set up their static buffer targets ahead of time. This implementation does not require that and will simply create the storage for the buffers as they are processed in the incoming IPC request. Static buffers are kept in an unordered_map keyed by their buffer id, and are written into the already-setup area of the request thread when responding an IPC request. This fixes a regression (crash) introduced in #2992. This PR introduces more warnings due to the [[deprecated]] attribute being added to void PushStaticBuffer(VAddr buffer_vaddr, size_t size, u8 buffer_id); and VAddr PopStaticBuffer(size_t* data_size); --- src/core/hle/ipc.h | 5 ++- src/core/hle/ipc_helpers.h | 56 ++++++++++++++++++---------- src/core/hle/kernel/hle_ipc.cpp | 42 +++++++++++++++++++++ src/core/hle/kernel/hle_ipc.h | 14 +++++++ src/core/hle/service/ac/ac.cpp | 18 ++++----- src/core/hle/service/apt/apt.cpp | 4 +- src/core/hle/service/frd/frd.cpp | 2 +- src/core/hle/service/fs/fs_user.cpp | 2 +- src/core/hle/service/ir/ir_user.cpp | 2 +- src/core/hle/service/nwm/nwm_uds.cpp | 8 ++-- 10 files changed, 113 insertions(+), 40 deletions(-) diff --git a/src/core/hle/ipc.h b/src/core/hle/ipc.h index 87ed85df6..24df91215 100644 --- a/src/core/hle/ipc.h +++ b/src/core/hle/ipc.h @@ -40,13 +40,16 @@ static const int kStaticBuffersOffset = 0x100; inline u32* GetStaticBuffers(const int offset = 0) { return GetCommandBuffer(kStaticBuffersOffset + offset); } -} +} // namespace Kernel namespace IPC { /// Size of the command buffer area, in 32-bit words. constexpr size_t COMMAND_BUFFER_LENGTH = 0x100 / sizeof(u32); +// Maximum number of static buffers per thread. +constexpr size_t MAX_STATIC_BUFFERS = 16; + // These errors are commonly returned by invalid IPC translations, so alias them here for // convenience. // TODO(yuriks): These will probably go away once translation is implemented inside the kernel. diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h index 7cb95cbac..61573b153 100644 --- a/src/core/hle/ipc_helpers.h +++ b/src/core/hle/ipc_helpers.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "core/hle/ipc.h" #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/hle_ipc.h" @@ -117,7 +118,8 @@ public: void PushCurrentPIDHandle(); - void PushStaticBuffer(VAddr buffer_vaddr, size_t size, u8 buffer_id); + [[deprecated]] void PushStaticBuffer(VAddr buffer_vaddr, size_t size, u8 buffer_id); + void PushStaticBuffer(const std::vector& buffer, u8 buffer_id); void PushMappedBuffer(VAddr buffer_vaddr, size_t size, MappedBufferPermissions perms); }; @@ -195,6 +197,16 @@ inline void RequestBuilder::PushStaticBuffer(VAddr buffer_vaddr, size_t size, u8 Push(buffer_vaddr); } +inline void RequestBuilder::PushStaticBuffer(const std::vector& buffer, u8 buffer_id) { + ASSERT_MSG(buffer_id < MAX_STATIC_BUFFERS, "Invalid static buffer id"); + + Push(StaticBufferDesc(buffer.size(), buffer_id)); + // This address will be replaced by the correct static buffer address during IPC translation. + Push(0xDEADC0DE); + + context->AddStaticBuffer(buffer_id, buffer); +} + inline void RequestBuilder::PushMappedBuffer(VAddr buffer_vaddr, size_t size, MappedBufferPermissions perms) { Push(MappedBufferDesc(size, perms)); @@ -295,17 +307,23 @@ public: * @brief Pops the static buffer vaddr * @return The virtual address of the buffer * @param[out] data_size If non-null, the pointed value will be set to the size of the data - * @param[out] useStaticBuffersToGetVaddr Indicates if we should read the vaddr from the static - * buffers (which is the correct thing to do, but no service presently implement it) instead of - * using the same value as the process who sent the request - * given by the source process * - * Static buffers must be set up before any IPC request using those is sent. + * In real services, static buffers must be set up before any IPC request using those is sent. * It is the duty of the process (usually services) to allocate and set up the receiving static - * buffer information + * buffer information. Our HLE services do not need to set up the buffers beforehand. * Please note that the setup uses virtual addresses. */ - VAddr PopStaticBuffer(size_t* data_size = nullptr, bool useStaticBuffersToGetVaddr = false); + [[deprecated]] VAddr PopStaticBuffer(size_t* data_size); + + /** + * @brief Pops a static buffer from the IPC request buffer. + * @return The buffer that was copied from the IPC request originator. + * + * In real services, static buffers must be set up before any IPC request using those is sent. + * It is the duty of the process (usually services) to allocate and set up the receiving static + * buffer information. Our HLE services do not need to set up the buffers beforehand. + */ + const std::vector& PopStaticBuffer(); /** * @brief Pops the mapped buffer vaddr @@ -451,21 +469,21 @@ inline std::tuple...> RequestParser::PopObjects() { std::index_sequence_for{}); } -inline VAddr RequestParser::PopStaticBuffer(size_t* data_size, bool useStaticBuffersToGetVaddr) { +inline VAddr RequestParser::PopStaticBuffer(size_t* data_size) { const u32 sbuffer_descriptor = Pop(); StaticBufferDescInfo bufferInfo{sbuffer_descriptor}; if (data_size != nullptr) *data_size = bufferInfo.size; - if (!useStaticBuffersToGetVaddr) - return Pop(); - else { - ASSERT_MSG(0, "remove the assert if multiprocess/IPC translation are implemented."); - // The buffer has already been copied to the static buffer by the kernel during - // translation - Pop(); // Pop the calling process buffer address - // and get the vaddr from the static buffers - return cmdbuf[(0x100 >> 2) + bufferInfo.buffer_id * 2 + 1]; - } + return Pop(); +} + +inline const std::vector& RequestParser::PopStaticBuffer() { + const u32 sbuffer_descriptor = Pop(); + // Pop the address from the incoming request buffer + Pop(); + + StaticBufferDescInfo buffer_info{sbuffer_descriptor}; + return context->GetStaticBuffer(buffer_info.buffer_id); } inline VAddr RequestParser::PopMappedBuffer(size_t* data_size, diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index 6020e9764..f906ccad0 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include #include "common/assert.h" #include "common/common_types.h" @@ -44,6 +45,16 @@ void HLERequestContext::ClearIncomingObjects() { request_handles.clear(); } +const std::vector& HLERequestContext::GetStaticBuffer(u8 buffer_id) const { + ASSERT_MSG(!static_buffers[buffer_id].empty(), "Empty static buffer!"); + return static_buffers[buffer_id]; +} + +void HLERequestContext::AddStaticBuffer(u8 buffer_id, std::vector data) { + ASSERT(static_buffers[buffer_id].empty() && !data.empty()); + static_buffers[buffer_id] = std::move(data); +} + ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* src_cmdbuf, Process& src_process, HandleTable& src_table) { @@ -84,6 +95,18 @@ ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* sr cmd_buf[i++] = src_process.process_id; break; } + case IPC::DescriptorType::StaticBuffer: { + VAddr source_address = src_cmdbuf[i]; + IPC::StaticBufferDescInfo buffer_info{descriptor}; + + // Copy the input buffer into our own vector and store it. + std::vector data(buffer_info.size); + Memory::ReadBlock(src_process, source_address, data.data(), data.size()); + + AddStaticBuffer(buffer_info.buffer_id, std::move(data)); + cmd_buf[i++] = source_address; + break; + } default: UNIMPLEMENTED_MSG("Unsupported handle translation: 0x%08X", descriptor); } @@ -124,6 +147,25 @@ ResultCode HLERequestContext::WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, P } break; } + case IPC::DescriptorType::StaticBuffer: { + IPC::StaticBufferDescInfo buffer_info{descriptor}; + + const auto& data = GetStaticBuffer(buffer_info.buffer_id); + + // Grab the address that the target thread set up to receive the response static buffer + // and write our data there. The static buffers area is located right after the command + // buffer area. + size_t static_buffer_offset = IPC::COMMAND_BUFFER_LENGTH + 2 * buffer_info.buffer_id; + IPC::StaticBufferDescInfo target_descriptor{dst_cmdbuf[static_buffer_offset]}; + VAddr target_address = dst_cmdbuf[static_buffer_offset + 1]; + + ASSERT_MSG(target_descriptor.size >= data.size(), "Static buffer data is too big"); + + Memory::WriteBlock(dst_process, target_address, data.data(), data.size()); + + dst_cmdbuf[i++] = target_address; + break; + } default: UNIMPLEMENTED_MSG("Unsupported handle translation: 0x%08X", descriptor); } diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index 35795fc1d..03a7a4deb 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -119,6 +119,18 @@ public: */ void ClearIncomingObjects(); + /** + * Retrieves the static buffer identified by the input buffer_id. The static buffer *must* have + * been created in PopulateFromIncomingCommandBuffer by way of an input StaticBuffer descriptor. + */ + const std::vector& GetStaticBuffer(u8 buffer_id) const; + + /** + * Sets up a static buffer that will be copied to the target process when the request is + * translated. + */ + void AddStaticBuffer(u8 buffer_id, std::vector data); + /// Populates this context with data from the requesting process/thread. ResultCode PopulateFromIncomingCommandBuffer(const u32_le* src_cmdbuf, Process& src_process, HandleTable& src_table); @@ -131,6 +143,8 @@ private: SharedPtr session; // TODO(yuriks): Check common usage of this and optimize size accordingly boost::container::small_vector, 8> request_handles; + // The static buffers will be created when the IPC request is translated. + std::array, IPC::MAX_STATIC_BUFFERS> static_buffers; }; } // namespace Kernel diff --git a/src/core/hle/service/ac/ac.cpp b/src/core/hle/service/ac/ac.cpp index bf9cad3d4..ee83dd1fd 100644 --- a/src/core/hle/service/ac/ac.cpp +++ b/src/core/hle/service/ac/ac.cpp @@ -2,6 +2,7 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include "common/common_types.h" #include "common/logging/log.h" #include "core/hle/ipc.h" @@ -19,17 +20,12 @@ namespace AC { void Module::Interface::CreateDefaultConfig(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp(ctx, 0x1, 0, 0); - std::size_t desc_size; - VAddr ac_config_addr = rp.PeekStaticBuffer(0, &desc_size); - - ASSERT_MSG(desc_size >= sizeof(Module::ACConfig), - "Output buffer size can't fit ACConfig structure"); - - Memory::WriteBlock(ac_config_addr, &ac->default_config, sizeof(ACConfig)); + std::vector buffer(sizeof(ACConfig)); + std::memcpy(buffer.data(), &ac->default_config, buffer.size()); IPC::RequestBuilder rb = rp.MakeBuilder(1, 2); rb.Push(RESULT_SUCCESS); - rb.PushStaticBuffer(ac_config_addr, sizeof(ACConfig), 0); + rb.PushStaticBuffer(std::move(buffer), 0); LOG_WARNING(Service_AC, "(STUBBED) called"); } @@ -106,7 +102,7 @@ void Module::Interface::GetWifiStatus(Kernel::HLERequestContext& ctx) { void Module::Interface::GetInfraPriority(Kernel::HLERequestContext& ctx) { IPC::RequestParser rp(ctx, 0x27, 0, 2); - VAddr ac_config = rp.PopStaticBuffer(); + const std::vector& ac_config = rp.PopStaticBuffer(); IPC::RequestBuilder rb = rp.MakeBuilder(2, 0); rb.Push(RESULT_SUCCESS); @@ -121,13 +117,13 @@ void Module::Interface::SetRequestEulaVersion(Kernel::HLERequestContext& ctx) { u32 major = rp.Pop(); u32 minor = rp.Pop(); - VAddr ac_config = rp.PopStaticBuffer(); + const std::vector& ac_config = rp.PopStaticBuffer(); // TODO(Subv): Copy over the input ACConfig to the stored ACConfig. IPC::RequestBuilder rb = rp.MakeBuilder(1, 2); rb.Push(RESULT_SUCCESS); - rb.PushStaticBuffer(ac_config, sizeof(ACConfig), 0); + rb.PushStaticBuffer(std::move(ac_config), 0); LOG_WARNING(Service_AC, "(STUBBED) called, major=%u, minor=%u", major, minor); } diff --git a/src/core/hle/service/apt/apt.cpp b/src/core/hle/service/apt/apt.cpp index e82a32868..cbebc6c9d 100644 --- a/src/core/hle/service/apt/apt.cpp +++ b/src/core/hle/service/apt/apt.cpp @@ -720,7 +720,7 @@ void AppletUtility(Service::Interface* self) { u32 utility_command = rp.Pop(); u32 input_size = rp.Pop(); u32 output_size = rp.Pop(); - VAddr input_addr = rp.PopStaticBuffer(); + VAddr input_addr = rp.PopStaticBuffer(nullptr); VAddr output_addr = rp.PeekStaticBuffer(0); @@ -823,7 +823,7 @@ void StartLibraryApplet(Service::Interface* self) { size_t buffer_size = rp.Pop(); Kernel::Handle handle = rp.PopHandle(); - VAddr buffer_addr = rp.PopStaticBuffer(); + VAddr buffer_addr = rp.PopStaticBuffer(nullptr); LOG_DEBUG(Service_APT, "called applet_id=%08X", static_cast(applet_id)); diff --git a/src/core/hle/service/frd/frd.cpp b/src/core/hle/service/frd/frd.cpp index 7ad7798da..b9687e5a6 100644 --- a/src/core/hle/service/frd/frd.cpp +++ b/src/core/hle/service/frd/frd.cpp @@ -113,7 +113,7 @@ void UnscrambleLocalFriendCode(Service::Interface* self) { IPC::RequestParser rp(Kernel::GetCommandBuffer(), 0x1C, 1, 2); const u32 friend_code_count = rp.Pop(); size_t in_buffer_size; - const VAddr scrambled_friend_codes = rp.PopStaticBuffer(&in_buffer_size, false); + const VAddr scrambled_friend_codes = rp.PopStaticBuffer(&in_buffer_size); ASSERT_MSG(in_buffer_size == (friend_code_count * scrambled_friend_code_size), "Wrong input buffer size"); diff --git a/src/core/hle/service/fs/fs_user.cpp b/src/core/hle/service/fs/fs_user.cpp index 4bd43d15d..d80d4665a 100644 --- a/src/core/hle/service/fs/fs_user.cpp +++ b/src/core/hle/service/fs/fs_user.cpp @@ -72,7 +72,7 @@ static void OpenFile(Service::Interface* self) { FileSys::Mode mode; mode.hex = rp.Pop(); u32 attributes = rp.Pop(); // TODO(Link Mauve): do something with those attributes. - VAddr filename_ptr = rp.PopStaticBuffer(); + VAddr filename_ptr = rp.PopStaticBuffer(nullptr); FileSys::Path file_path(filename_type, filename_size, filename_ptr); LOG_DEBUG(Service_FS, "path=%s, mode=%u attrs=%u", file_path.DebugStr().c_str(), mode.hex, diff --git a/src/core/hle/service/ir/ir_user.cpp b/src/core/hle/service/ir/ir_user.cpp index fbdf7a465..1c0af1572 100644 --- a/src/core/hle/service/ir/ir_user.cpp +++ b/src/core/hle/service/ir/ir_user.cpp @@ -433,7 +433,7 @@ static void FinalizeIrNop(Interface* self) { static void SendIrNop(Interface* self) { IPC::RequestParser rp(Kernel::GetCommandBuffer(), 0x0D, 1, 2); const u32 size = rp.Pop(); - const VAddr address = rp.PopStaticBuffer(); + const VAddr address = rp.PopStaticBuffer(nullptr); std::vector buffer(size); Memory::ReadBlock(address, buffer.data(), size); diff --git a/src/core/hle/service/nwm/nwm_uds.cpp b/src/core/hle/service/nwm/nwm_uds.cpp index 1bf13a296..47b7e06da 100644 --- a/src/core/hle/service/nwm/nwm_uds.cpp +++ b/src/core/hle/service/nwm/nwm_uds.cpp @@ -771,9 +771,9 @@ static void BeginHostingNetwork(Interface* self) { const u32 passphrase_size = rp.Pop(); size_t desc_size; - const VAddr network_info_address = rp.PopStaticBuffer(&desc_size, false); + const VAddr network_info_address = rp.PopStaticBuffer(&desc_size); ASSERT(desc_size == sizeof(NetworkInfo)); - const VAddr passphrase_address = rp.PopStaticBuffer(&desc_size, false); + const VAddr passphrase_address = rp.PopStaticBuffer(&desc_size); ASSERT(desc_size == passphrase_size); // TODO(Subv): Store the passphrase and verify it when attempting a connection. @@ -907,7 +907,7 @@ static void SendTo(Interface* self) { u32 flags = rp.Pop(); size_t desc_size; - const VAddr input_address = rp.PopStaticBuffer(&desc_size, false); + const VAddr input_address = rp.PopStaticBuffer(&desc_size); ASSERT(desc_size >= data_size); IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); @@ -1093,7 +1093,7 @@ static void SetApplicationData(Interface* self) { u32 size = rp.Pop(); size_t desc_size; - const VAddr address = rp.PopStaticBuffer(&desc_size, false); + const VAddr address = rp.PopStaticBuffer(&desc_size); ASSERT(desc_size == size); LOG_DEBUG(Service_NWM, "called"); From 35a61ab053afa17e7e7678e7b6bd9a7492de7f4d Mon Sep 17 00:00:00 2001 From: Subv Date: Mon, 6 Nov 2017 20:32:07 -0500 Subject: [PATCH 2/2] HLE/Tests: Added tests for the StaticBuffer IPC HLE translation. --- src/tests/core/hle/kernel/hle_ipc.cpp | 74 +++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/src/tests/core/hle/kernel/hle_ipc.cpp b/src/tests/core/hle/kernel/hle_ipc.cpp index 52336d027..64b06cc8b 100644 --- a/src/tests/core/hle/kernel/hle_ipc.cpp +++ b/src/tests/core/hle/kernel/hle_ipc.cpp @@ -116,25 +116,58 @@ TEST_CASE("HLERequestContext::PopulateFromIncomingCommandBuffer", "[core][kernel REQUIRE(context.CommandBuffer()[2] == process->process_id); } + SECTION("translates StaticBuffer descriptors") { + auto buffer = std::make_shared>(Memory::PAGE_SIZE); + std::fill(buffer->begin(), buffer->end(), 0xAB); + + VAddr target_address = 0x10000000; + auto result = process->vm_manager.MapMemoryBlock(target_address, buffer, 0, buffer->size(), + MemoryState::Private); + REQUIRE(result.Code() == RESULT_SUCCESS); + + const u32_le input[]{ + IPC::MakeHeader(0, 0, 2), IPC::StaticBufferDesc(buffer->size(), 0), target_address, + }; + + context.PopulateFromIncomingCommandBuffer(input, *process, handle_table); + + CHECK(context.GetStaticBuffer(0) == *buffer); + + REQUIRE(process->vm_manager.UnmapRange(target_address, buffer->size()) == RESULT_SUCCESS); + } + SECTION("translates mixed params") { + auto buffer = std::make_shared>(Memory::PAGE_SIZE); + std::fill(buffer->begin(), buffer->end(), 0xCE); + + VAddr target_address = 0x10000000; + auto result = process->vm_manager.MapMemoryBlock(target_address, buffer, 0, buffer->size(), + MemoryState::Private); + REQUIRE(result.Code() == RESULT_SUCCESS); + auto a = MakeObject(); const u32_le input[]{ - IPC::MakeHeader(0, 2, 4), + IPC::MakeHeader(0, 2, 6), 0x12345678, 0xABCDEF00, IPC::MoveHandleDesc(1), handle_table.Create(a).Unwrap(), IPC::CallingPidDesc(), 0, + IPC::StaticBufferDesc(buffer->size(), 0), + target_address, }; context.PopulateFromIncomingCommandBuffer(input, *process, handle_table); auto* output = context.CommandBuffer(); - REQUIRE(output[1] == 0x12345678); - REQUIRE(output[2] == 0xABCDEF00); - REQUIRE(context.GetIncomingHandle(output[4]) == a); - REQUIRE(output[6] == process->process_id); + CHECK(output[1] == 0x12345678); + CHECK(output[2] == 0xABCDEF00); + CHECK(context.GetIncomingHandle(output[4]) == a); + CHECK(output[6] == process->process_id); + CHECK(context.GetStaticBuffer(0) == *buffer); + + REQUIRE(process->vm_manager.UnmapRange(target_address, buffer->size()) == RESULT_SUCCESS); } } @@ -211,6 +244,37 @@ TEST_CASE("HLERequestContext::WriteToOutgoingCommandBuffer", "[core][kernel]") { REQUIRE(handle_table.GetGeneric(output[3]) == b); REQUIRE(handle_table.GetGeneric(output[5]) == c); } + + SECTION("translates StaticBuffer descriptors") { + std::vector input_buffer(Memory::PAGE_SIZE); + std::fill(input_buffer.begin(), input_buffer.end(), 0xAB); + + context.AddStaticBuffer(0, input_buffer); + + auto output_buffer = std::make_shared>(Memory::PAGE_SIZE); + VAddr target_address = 0x10000000; + auto result = process->vm_manager.MapMemoryBlock( + target_address, output_buffer, 0, output_buffer->size(), MemoryState::Private); + REQUIRE(result.Code() == RESULT_SUCCESS); + + input[0] = IPC::MakeHeader(0, 0, 2); + input[1] = IPC::StaticBufferDesc(input_buffer.size(), 0); + input[2] = target_address; + + // An entire command buffer plus enough space for one static buffer descriptor and its + // target address + std::array output_cmdbuff; + // Set up the output StaticBuffer + output_cmdbuff[IPC::COMMAND_BUFFER_LENGTH] = + IPC::StaticBufferDesc(output_buffer->size(), 0); + output_cmdbuff[IPC::COMMAND_BUFFER_LENGTH + 1] = target_address; + + context.WriteToOutgoingCommandBuffer(output_cmdbuff.data(), *process, handle_table); + + CHECK(*output_buffer == input_buffer); + REQUIRE(process->vm_manager.UnmapRange(target_address, output_buffer->size()) == + RESULT_SUCCESS); + } } } // namespace Kernel