From b3517357e1eabd949201f22a19421bdd7defc499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=97=B1=20PixelyIon?= Date: Fri, 15 Nov 2019 00:36:38 +0530 Subject: [PATCH] Fix: Crash on reading domain header from Control message The application would crash if there was a control request from a domain message. As it would still try to read the domain header in the message. --- app/src/main/cpp/skyline/kernel/ipc.cpp | 62 ++++++----- app/src/main/cpp/skyline/kernel/ipc.h | 103 +++++++++--------- app/src/main/cpp/skyline/loader/loader.h | 2 +- .../main/cpp/skyline/services/serviceman.cpp | 6 +- build.gradle | 2 +- 5 files changed, 88 insertions(+), 87 deletions(-) diff --git a/app/src/main/cpp/skyline/kernel/ipc.cpp b/app/src/main/cpp/skyline/kernel/ipc.cpp index fe0a48e7..4d83ff48 100644 --- a/app/src/main/cpp/skyline/kernel/ipc.cpp +++ b/app/src/main/cpp/skyline/kernel/ipc.cpp @@ -9,20 +9,20 @@ namespace skyline::kernel::ipc { header = reinterpret_cast(currPtr); currPtr += sizeof(CommandHeader); - if (header->handle_desc) { + if (header->handleDesc) { handleDesc = reinterpret_cast(currPtr); - currPtr += sizeof(HandleDescriptor) + (handleDesc->send_pid ? sizeof(u64) : 0); - for (uint index = 0; handleDesc->copy_count > index; index++) { + currPtr += sizeof(HandleDescriptor) + (handleDesc->sendPid ? sizeof(u64) : 0); + for (uint index = 0; handleDesc->copyCount > index; index++) { copyHandles.push_back(*reinterpret_cast(currPtr)); currPtr += sizeof(handle_t); } - for (uint index = 0; handleDesc->move_count > index; index++) { + for (uint index = 0; handleDesc->moveCount > index; index++) { moveHandles.push_back(*reinterpret_cast(currPtr)); currPtr += sizeof(handle_t); } } - for (uint index = 0; header->x_no > index; index++) { + for (uint index = 0; header->Xno > index; index++) { auto bufX = reinterpret_cast(currPtr); if (bufX->Address()) { vecBufX.push_back(bufX); @@ -31,7 +31,7 @@ namespace skyline::kernel::ipc { currPtr += sizeof(BufferDescriptorX); } - for (uint index = 0; header->a_no > index; index++) { + for (uint index = 0; header->Ano > index; index++) { auto bufA = reinterpret_cast(currPtr); if (bufA->Address()) { vecBufA.push_back(bufA); @@ -40,7 +40,7 @@ namespace skyline::kernel::ipc { currPtr += sizeof(BufferDescriptorABW); } - for (uint index = 0; header->b_no > index; index++) { + for (uint index = 0; header->Bno > index; index++) { auto bufB = reinterpret_cast(currPtr); if (bufB->Address()) { vecBufB.push_back(bufB); @@ -49,7 +49,7 @@ namespace skyline::kernel::ipc { currPtr += sizeof(BufferDescriptorABW); } - for (uint index = 0; header->w_no > index; index++) { + for (uint index = 0; header->Wno > index; index++) { auto bufW = reinterpret_cast(currPtr); if (bufW->Address()) { vecBufW.push_back(bufW); @@ -61,7 +61,7 @@ namespace skyline::kernel::ipc { u64 padding = ((((reinterpret_cast(currPtr) - reinterpret_cast(tls.data())) - 1U) & ~(constant::IpcPaddingSum - 1U)) + constant::IpcPaddingSum + (reinterpret_cast(tls.data()) - reinterpret_cast(currPtr))); // Calculate the amount of padding at the front currPtr += padding; - if (isDomain) { + if (isDomain && (header->type == CommandType::Request)) { domain = reinterpret_cast(currPtr); currPtr += sizeof(DomainHeaderRequest); @@ -69,10 +69,10 @@ namespace skyline::kernel::ipc { currPtr += sizeof(PayloadHeader); cmdArg = currPtr; - cmdArgSz = domain->payload_sz - sizeof(PayloadHeader); - currPtr += domain->payload_sz; + cmdArgSz = domain->payloadSz - sizeof(PayloadHeader); + currPtr += domain->payloadSz; - for (uint index = 0; domain->input_count > index; index++) { + for (uint index = 0; domain->inputCount > index; index++) { domainObjects.push_back(*reinterpret_cast(currPtr)); currPtr += sizeof(handle_t); } @@ -81,21 +81,21 @@ namespace skyline::kernel::ipc { currPtr += sizeof(PayloadHeader); cmdArg = currPtr; - cmdArgSz = (header->raw_sz * sizeof(u32)) - (constant::IpcPaddingSum + sizeof(PayloadHeader)); + cmdArgSz = (header->rawSize * sizeof(u32)) - (constant::IpcPaddingSum + sizeof(PayloadHeader)); currPtr += cmdArgSz; } - if (payload->magic != constant::SfciMagic && header->type != static_cast(CommandType::Control)) + if (payload->magic != constant::SfciMagic && header->type != CommandType::Control) state.logger->Debug("Unexpected Magic in PayloadHeader: 0x{:X}", u32(payload->magic)); currPtr += constant::IpcPaddingSum - padding; - if (header->c_flag == static_cast(BufferCFlag::SingleDescriptor)) { + if (header->cFlag == BufferCFlag::SingleDescriptor) { auto bufC = reinterpret_cast(currPtr); vecBufC.push_back(bufC); state.logger->Debug("Buf C: AD: 0x{:X} SZ: 0x{:X}", u64(bufC->address), u16(bufC->size)); - } else if (header->c_flag > static_cast(BufferCFlag::SingleDescriptor)) { - for (uint index = 0; (header->c_flag - 2) > index; index++) { // (c_flag - 2) C descriptors are present + } else if (header->cFlag > BufferCFlag::SingleDescriptor) { + for (uint index = 0; (static_cast(header->cFlag) - 2) > index; index++) { // (cFlag - 2) C descriptors are present auto bufC = reinterpret_cast(currPtr); if (bufC->address) { vecBufC.push_back(bufC); @@ -105,12 +105,14 @@ namespace skyline::kernel::ipc { } } - state.logger->Debug("Header: X No: {}, A No: {}, B No: {}, W No: {}, C No: {}, Raw Size: {}", u8(header->x_no), u8(header->a_no), u8(header->b_no), u8(header->w_no), u8(vecBufC.size()), u64(cmdArgSz)); - if (header->handle_desc) - state.logger->Debug("Handle Descriptor: Send PID: {}, Copy Count: {}, Move Count: {}", bool(handleDesc->send_pid), u32(handleDesc->copy_count), u32(handleDesc->move_count)); - if (isDomain) - state.logger->Debug("Domain Header: Command: {}, Input Object Count: {}, Object ID: 0x{:X}", domain->command, domain->input_count, domain->object_id); - state.logger->Debug("Data Payload: Command ID: 0x{:X}", u32(payload->value)); + if (header->type == CommandType::Request) { + state.logger->Debug("Header: X No: {}, A No: {}, B No: {}, W No: {}, C No: {}, Raw Size: {}", u8(header->Xno), u8(header->Ano), u8(header->Bno), u8(header->Wno), u8(vecBufC.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)); + if (isDomain) + state.logger->Debug("Domain Header: Command: {}, Input Object Count: {}, Object ID: 0x{:X}", domain->command, domain->inputCount, domain->objectId); + state.logger->Debug("Command ID: 0x{:X}", u32(payload->value)); + } } IpcResponse::IpcResponse(bool isDomain, const DeviceState &state) : isDomain(isDomain), state(state) {} @@ -119,14 +121,14 @@ namespace skyline::kernel::ipc { std::array tls{}; u8 *currPtr = tls.data(); auto header = reinterpret_cast(currPtr); - header->raw_sz = static_cast((sizeof(PayloadHeader) + argVec.size() + (domainObjects.size() * sizeof(handle_t)) + constant::IpcPaddingSum + (isDomain ? sizeof(DomainHeaderRequest) : 0)) / sizeof(u32)); // Size is in 32-bit units because Nintendo - header->handle_desc = (!copyHandles.empty() || !moveHandles.empty()); + header->rawSize = static_cast((sizeof(PayloadHeader) + argVec.size() + (domainObjects.size() * sizeof(handle_t)) + constant::IpcPaddingSum + (isDomain ? sizeof(DomainHeaderRequest) : 0)) / sizeof(u32)); // Size is in 32-bit units because Nintendo + header->handleDesc = (!copyHandles.empty() || !moveHandles.empty()); currPtr += sizeof(CommandHeader); - if (header->handle_desc) { + if (header->handleDesc) { auto handleDesc = reinterpret_cast(currPtr); - handleDesc->copy_count = static_cast(copyHandles.size()); - handleDesc->move_count = static_cast(moveHandles.size()); + handleDesc->copyCount = static_cast(copyHandles.size()); + handleDesc->moveCount = static_cast(moveHandles.size()); currPtr += sizeof(HandleDescriptor); for (unsigned int copyHandle : copyHandles) { @@ -145,7 +147,7 @@ namespace skyline::kernel::ipc { if (isDomain) { auto domain = reinterpret_cast(currPtr); - domain->output_count = static_cast(domainObjects.size()); + domain->outputCount = static_cast(domainObjects.size()); currPtr += sizeof(DomainHeaderResponse); } @@ -165,7 +167,7 @@ namespace skyline::kernel::ipc { } } - state.logger->Debug("Output: Raw Size: {}, Command ID: 0x{:X}, Copy Handles: {}, Move Handles: {}", u32(header->raw_sz), u32(payload->value), copyHandles.size(), moveHandles.size()); + state.logger->Debug("Output: Raw Size: {}, Command ID: 0x{:X}, Copy Handles: {}, Move Handles: {}", u32(header->rawSize), u32(payload->value), copyHandles.size(), moveHandles.size()); state.thisProcess->WriteMemory(tls.data(), state.thisThread->tls, constant::TlsIpcSize); } diff --git a/app/src/main/cpp/skyline/kernel/ipc.h b/app/src/main/cpp/skyline/kernel/ipc.h index b0553004..db04bcb5 100644 --- a/app/src/main/cpp/skyline/kernel/ipc.h +++ b/app/src/main/cpp/skyline/kernel/ipc.h @@ -4,22 +4,6 @@ #include namespace skyline::kernel::ipc { - /** - * @brief This bit-field structure holds the header of an IPC command. (https://switchbrew.org/wiki/IPC_Marshalling#IPC_Command_Structure) - */ - struct CommandHeader { - u16 type : 16; - u8 x_no : 4; - u8 a_no : 4; - u8 b_no : 4; - u8 w_no : 4; - u32 raw_sz : 10; - u8 c_flag : 4; - u32 : 17; - bool handle_desc : 1; - }; - static_assert(sizeof(CommandHeader) == 8); - /** * @brief This reflects the value in CommandStruct::type */ @@ -34,14 +18,30 @@ namespace skyline::kernel::ipc { None = 0, InlineDescriptor = 1, SingleDescriptor = 2 }; + /** + * @brief This bit-field structure holds the header of an IPC command. (https://switchbrew.org/wiki/IPC_Marshalling#IPC_Command_Structure) + */ + struct CommandHeader { + CommandType type : 16; + u8 Xno : 4; + u8 Ano : 4; + u8 Bno : 4; + u8 Wno : 4; + u32 rawSize : 10; + BufferCFlag cFlag : 4; + u32 : 17; + bool handleDesc : 1; + }; + static_assert(sizeof(CommandHeader) == 8); + /** * @brief This bit-field structure holds the handle descriptor of a received IPC command. (https://switchbrew.org/wiki/IPC_Marshalling#Handle_descriptor) */ struct HandleDescriptor { - bool send_pid : 1; - u32 copy_count : 4; - u32 move_count : 4; - u32 : 23; + bool sendPid : 1; + u32 copyCount : 4; + u32 moveCount : 4; + u32 : 23; }; static_assert(sizeof(HandleDescriptor) == 4); @@ -50,9 +50,9 @@ namespace skyline::kernel::ipc { */ struct DomainHeaderRequest { u8 command; - u8 input_count; - u16 payload_sz; - u32 object_id; + u8 inputCount; + u16 payloadSz; + u32 objectId; u32 : 32; u32 token; }; @@ -69,7 +69,7 @@ namespace skyline::kernel::ipc { * @brief This bit-field structure holds the domain's header of an IPC response command. (https://switchbrew.org/wiki/IPC_Marshalling#Domains) */ struct DomainHeaderResponse { - u32 output_count; + u32 outputCount; u32 : 32; u64 : 64; }; @@ -98,27 +98,27 @@ namespace skyline::kernel::ipc { * @brief This is a buffer descriptor for X buffers: https://switchbrew.org/wiki/IPC_Marshalling#Buffer_descriptor_X_.22Pointer.22 */ struct BufferDescriptorX { - u16 counter_0_5 : 6; - u16 address_36_38 : 3; - u16 counter_9_11 : 3; - u16 address_32_35 : 4; - u16 size : 16; - u32 address_0_31 : 32; + u16 counter0_5 : 6; + u16 address36_38 : 3; + u16 counter9_11 : 3; + u16 address32_35 : 4; + u16 size : 16; + u32 address0_31 : 32; BufferDescriptorX(u64 address, u16 counter, u16 size) : size(size) { - address_0_31 = static_cast(address & 0x7FFFFFFF80000000); - address_32_35 = static_cast(address & 0x78000000); - address_36_38 = static_cast(address & 0x7000000); - counter_0_5 = static_cast(address & 0x7E00); - counter_9_11 = static_cast(address & 0x38); + address0_31 = static_cast(address & 0x7FFFFFFF80000000); + address32_35 = static_cast(address & 0x78000000); + address36_38 = static_cast(address & 0x7000000); + counter0_5 = static_cast(address & 0x7E00); + counter9_11 = static_cast(address & 0x38); } inline u64 Address() const { - return static_cast(address_0_31) | static_cast(address_32_35) << 32 | static_cast(address_36_38) << 36; + return static_cast(address0_31) | static_cast(address32_35) << 32 | static_cast(address36_38) << 36; } inline u16 Counter() const { - return static_cast(counter_0_5) | static_cast(counter_9_11) << 9; + return static_cast(counter0_5) | static_cast(counter9_11) << 9; } }; @@ -128,30 +128,30 @@ namespace skyline::kernel::ipc { * @brief This is a buffer descriptor for A/B/W buffers: https://switchbrew.org/wiki/IPC_Marshalling#Buffer_descriptor_A.2FB.2FW_.22Send.22.2F.22Receive.22.2F.22Exchange.22 */ struct BufferDescriptorABW { - u32 size_0_31 : 32; - u32 address_0_31 : 32; - u8 flags : 2; - u8 address_36_38 : 3; - u32 : 19; - u8 size_32_35 : 4; - u8 address_32_35 : 4; + u32 size0_31 : 32; + u32 address0_31 : 32; + u8 flags : 2; + u8 address36_38 : 3; + u32 : 19; + u8 size32_35 : 4; + u8 address32_35 : 4; BufferDescriptorABW(u64 address, u64 size) { - address_0_31 = static_cast(address & 0x7FFFFFFF80000000); - address_32_35 = static_cast(address & 0x78000000); - address_36_38 = static_cast(address & 0x7000000); - size_0_31 = static_cast(size & 0x7FFFFFFF80000000); - size_32_35 = static_cast(size & 0x78000000); + address0_31 = static_cast(address & 0x7FFFFFFF80000000); + address32_35 = static_cast(address & 0x78000000); + address36_38 = static_cast(address & 0x7000000); + size0_31 = static_cast(size & 0x7FFFFFFF80000000); + size32_35 = static_cast(size & 0x78000000); } std::vector Read(const DeviceState &state); inline u64 Address() const { - return static_cast(address_0_31) | static_cast(address_32_35) << 32 | static_cast(address_36_38) << 36; + return static_cast(address0_31) | static_cast(address32_35) << 32 | static_cast(address36_38) << 36; } inline u64 Size() const { - return static_cast(size_0_31) | static_cast(size_32_35) << 32; + return static_cast(size0_31) | static_cast(size32_35) << 32; } }; @@ -166,7 +166,6 @@ namespace skyline::kernel::ipc { BufferDescriptorC(u64 address, u16 size) : address(address), size(size) {} }; - static_assert(sizeof(BufferDescriptorC) == 8); /** diff --git a/app/src/main/cpp/skyline/loader/loader.h b/app/src/main/cpp/skyline/loader/loader.h index be40f85e..20d6500b 100644 --- a/app/src/main/cpp/skyline/loader/loader.h +++ b/app/src/main/cpp/skyline/loader/loader.h @@ -50,7 +50,7 @@ namespace skyline::loader { public: /** - * @param file_path_ The path to the ROM file + * @param filePath The path to the ROM file */ Loader(std::string &filePath) : filePath(filePath), file(filePath, std::ios::binary | std::ios::beg) {} diff --git a/app/src/main/cpp/skyline/services/serviceman.cpp b/app/src/main/cpp/skyline/services/serviceman.cpp index 84839fa1..201df390 100644 --- a/app/src/main/cpp/skyline/services/serviceman.cpp +++ b/app/src/main/cpp/skyline/services/serviceman.cpp @@ -172,14 +172,14 @@ namespace skyline::kernel::service { case ipc::CommandType::RequestWithContext: if (session->isDomain) { try { - auto service = session->domainTable.at(request.domain->object_id); + auto service = session->domainTable.at(request.domain->objectId); switch (static_cast(request.domain->command)) { case ipc::DomainCommand::SendMessage: service->HandleRequest(*session, request, response); break; case ipc::DomainCommand::CloseVHandle: serviceVec.erase(std::remove(serviceVec.begin(), serviceVec.end(), service), serviceVec.end()); - session->domainTable.erase(request.domain->object_id); + session->domainTable.erase(request.domain->objectId); break; } } catch (std::out_of_range &) { @@ -193,7 +193,7 @@ namespace skyline::kernel::service { case ipc::CommandType::Control: case ipc::CommandType::ControlWithContext: - state.logger->Debug("Control IPC Message: {}", request.payload->value); + state.logger->Debug("Control IPC Message: 0x{:X}", request.payload->value); switch (static_cast(request.payload->value)) { case ipc::ControlCommand::ConvertCurrentObjectToDomain: response.WriteValue(session->ConvertDomain()); diff --git a/build.gradle b/build.gradle index 4c880263..753b4d73 100644 --- a/build.gradle +++ b/build.gradle @@ -7,7 +7,7 @@ buildscript { } dependencies { - classpath 'com.android.tools.build:gradle:3.5.1' + classpath 'com.android.tools.build:gradle:3.5.2' // NOTE: Do not place your application dependencies here; they belong // in the individual module build.gradle files