From c423a66020aa593a929dd28722f6d89c63d790b1 Mon Sep 17 00:00:00 2001 From: Billy Laws Date: Tue, 23 Jun 2020 19:49:06 +0100 Subject: [PATCH] Fixes for control IPC (#57) * Correctly handle -WithContext IPC Requests They should be treated the same as the non WithContext variants. * Only send domain data on non-control IPC responses Control IPC doesn't make use of domains so we shouldn't send extra data in the response. * Add the IStorage implementation to CMakeLists --- app/CMakeLists.txt | 1 + app/src/main/cpp/skyline/kernel/ipc.cpp | 10 +++++----- app/src/main/cpp/skyline/kernel/ipc.h | 7 +++---- app/src/main/cpp/skyline/services/serviceman.cpp | 11 +++++------ 4 files changed, 14 insertions(+), 15 deletions(-) diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index e8effd35..d4488751 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -83,6 +83,7 @@ add_library(skyline SHARED ${source_DIR}/skyline/services/timesrv/ITimeZoneService.cpp ${source_DIR}/skyline/services/fssrv/IFileSystemProxy.cpp ${source_DIR}/skyline/services/fssrv/IFileSystem.cpp + ${source_DIR}/skyline/services/fssrv/IStorage.cpp ${source_DIR}/skyline/services/nvdrv/INvDrvServices.cpp ${source_DIR}/skyline/services/nvdrv/devices/nvmap.cpp ${source_DIR}/skyline/services/nvdrv/devices/nvhost_ctrl_gpu.cpp diff --git a/app/src/main/cpp/skyline/kernel/ipc.cpp b/app/src/main/cpp/skyline/kernel/ipc.cpp index 44dc8998..f0ce5b46 100644 --- a/app/src/main/cpp/skyline/kernel/ipc.cpp +++ b/app/src/main/cpp/skyline/kernel/ipc.cpp @@ -76,7 +76,7 @@ namespace skyline::kernel::ipc { auto padding = util::AlignUp(offset, constant::IpcPaddingSum) - offset; // Calculate the amount of padding at the front pointer += padding; - if (isDomain && (header->type == CommandType::Request)) { + if (isDomain && (header->type == CommandType::Request || header->type == CommandType::RequestWithContext)) { domain = reinterpret_cast(pointer); pointer += sizeof(DomainHeaderRequest); @@ -102,7 +102,7 @@ namespace skyline::kernel::ipc { payloadOffset = cmdArg; - if (payload->magic != util::MakeMagic("SFCI") && header->type != CommandType::Control) // SFCI is the magic in received IPC messages + if (payload->magic != util::MakeMagic("SFCI") && (header->type != CommandType::Control && header->type != CommandType::ControlWithContext)) // SFCI is the magic in received IPC messages state.logger->Debug("Unexpected Magic in PayloadHeader: 0x{:X}", u32(payload->magic)); pointer += constant::IpcPaddingSum - padding; @@ -124,7 +124,7 @@ namespace skyline::kernel::ipc { } } - if (header->type == CommandType::Request) { + if (header->type == CommandType::Request || header->type == CommandType::RequestWithContext) { state.logger->Debug("Header: Input No: {}, Output No: {}, Raw Size: {}", inputBuf.size(), outputBuf.size(), u64(cmdArgSz)); if (header->handleDesc) state.logger->Debug("Handle Descriptor: Send PID: {}, Copy Count: {}, Move Count: {}", bool(handleDesc->sendPid), u32(handleDesc->copyCount), u32(handleDesc->moveCount)); @@ -134,9 +134,9 @@ namespace skyline::kernel::ipc { } } - IpcResponse::IpcResponse(bool isDomain, const DeviceState &state) : isDomain(isDomain), state(state) {} + IpcResponse::IpcResponse(const DeviceState &state) : state(state) {} - void IpcResponse::WriteResponse() { + void IpcResponse::WriteResponse(bool isDomain) { auto tls = state.process->GetPointer(state.thread->tls); u8 *pointer = tls; diff --git a/app/src/main/cpp/skyline/kernel/ipc.h b/app/src/main/cpp/skyline/kernel/ipc.h index 2f7eb03b..6eab6634 100644 --- a/app/src/main/cpp/skyline/kernel/ipc.h +++ b/app/src/main/cpp/skyline/kernel/ipc.h @@ -310,8 +310,6 @@ namespace skyline { std::vector payload; //!< This holds all of the contents to be pushed to the payload public: - bool nWrite{}; //!< This is to signal the IPC handler to not write this, as it will be manually written - bool isDomain{}; //!< If this is a domain request u32 errorCode{}; //!< The error code to respond with, it is 0 (Success) by default std::vector copyHandles; //!< A vector of handles to copy std::vector moveHandles; //!< A vector of handles to move @@ -321,7 +319,7 @@ namespace skyline { * @param isDomain If the following request is a domain request * @param state The state of the device */ - IpcResponse(bool isDomain, const DeviceState &state); + IpcResponse(const DeviceState &state); /** * @brief Writes an object to the payload @@ -350,8 +348,9 @@ namespace skyline { /** * @brief Writes this IpcResponse object's contents into TLS + * @param isDomain Indicates if this is a domain response */ - void WriteResponse(); + void WriteResponse(bool isDomain); }; } } diff --git a/app/src/main/cpp/skyline/services/serviceman.cpp b/app/src/main/cpp/skyline/services/serviceman.cpp index 4d924b15..4b9e53b2 100644 --- a/app/src/main/cpp/skyline/services/serviceman.cpp +++ b/app/src/main/cpp/skyline/services/serviceman.cpp @@ -82,7 +82,7 @@ namespace skyline::service { std::lock_guard serviceGuard(mutex); auto serviceObject = CreateService(ServiceString.at(serviceName)); KHandle handle{}; - if (response.isDomain) { + if (session.isDomain) { session.domainTable[++session.handleIndex] = serviceObject; response.domainObjects.push_back(session.handleIndex); handle = session.handleIndex; @@ -97,7 +97,7 @@ namespace skyline::service { void ServiceManager::RegisterService(std::shared_ptr serviceObject, type::KSession &session, ipc::IpcResponse &response, bool submodule) { // NOLINT(performance-unnecessary-value-param) std::lock_guard serviceGuard(mutex); KHandle handle{}; - if (response.isDomain) { + if (session.isDomain) { session.domainTable[session.handleIndex] = serviceObject; response.domainObjects.push_back(session.handleIndex); handle = session.handleIndex++; @@ -131,7 +131,7 @@ namespace skyline::service { if (session->serviceStatus == type::KSession::ServiceStatus::Open) { ipc::IpcRequest request(session->isDomain, state); - ipc::IpcResponse response(session->isDomain, state); + ipc::IpcResponse response(state); switch (request.header->type) { case ipc::CommandType::Request: @@ -154,8 +154,7 @@ namespace skyline::service { } else { session->serviceObject->HandleRequest(*session, request, response); } - if (!response.nWrite) - response.WriteResponse(); + response.WriteResponse(session->isDomain); break; case ipc::CommandType::Control: case ipc::CommandType::ControlWithContext: @@ -174,7 +173,7 @@ namespace skyline::service { default: throw exception("Unknown Control Command: {}", request.payload->value); } - response.WriteResponse(); + response.WriteResponse(false); break; case ipc::CommandType::Close: state.logger->Debug("Closing Session");