From da5a7fcc63e75e9319fc0010dbc4ccaece5427d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Tue, 27 Jun 2017 15:04:47 +0200 Subject: [PATCH] IOS/ES: Fix content table handling This is larger than I thought I would be, but unfortunately it's quite hard to split fixes like this when the handling is wrong in tons of different places. The content table is limited in size. It can only hold 16 entries. Three consequences: * Since the table cannot grow indefinitely, instead of using a std::map we use a std::array as we should. * Remove a hack where the CFD was cleared back to 0 on IPC close (wtf?) * The CFD now doesn't keep increasing to infinity. It's unknown if this would fix anything at all, but some issues in the past were caused by CFDs being excessively large. Other minor changes: * Simplify save state logic. * Keep track of the UID like ES does. Not sure how useful this is, but we can do this very easily so why not. * Remove the guesswork and use the actual error codes. * Add more error checking to make Dolphin less likely to crash. Something that should be done in the future: deduplicate the filesystem logic. Something that takes one line in the actual ES code takes 10+ lines in our implementation... while duplicating the FS logic... This will likely harder to fix though, so I'm leaving that for another time. --- Source/Core/Core/IOS/ES/ES.cpp | 35 +--- Source/Core/Core/IOS/ES/ES.h | 20 +- Source/Core/Core/IOS/ES/TitleContents.cpp | 211 +++++++++++----------- Source/Core/Core/State.cpp | 2 +- 4 files changed, 125 insertions(+), 143 deletions(-) diff --git a/Source/Core/Core/IOS/ES/ES.cpp b/Source/Core/Core/IOS/ES/ES.cpp index 9cf025b8ae..88aa3e28c0 100644 --- a/Source/Core/Core/IOS/ES/ES.cpp +++ b/Source/Core/Core/IOS/ES/ES.cpp @@ -339,34 +339,11 @@ void ES::DoState(PointerWrap& p) { Device::DoState(p); p.Do(s_content_file); - p.Do(m_AccessIdentID); + p.Do(m_content_table); s_title_context.DoState(p); for (auto& context : m_contexts) context.DoState(p); - - u32 Count = (u32)(m_ContentAccessMap.size()); - p.Do(Count); - - if (p.GetMode() == PointerWrap::MODE_READ) - { - for (u32 i = 0; i < Count; i++) - { - u32 cfd = 0; - OpenedContent content; - p.Do(cfd); - p.Do(content); - cfd = OpenTitleContent(cfd, content.m_title_id, content.m_content.index); - } - } - else - { - for (const auto& pair : m_ContentAccessMap) - { - p.Do(pair.first); - p.Do(pair.second); - } - } } ES::ContextArray::iterator ES::FindActiveContext(s32 fd) @@ -403,10 +380,6 @@ ReturnCode ES::Close(u32 fd) context->active = false; context->ipc_fd = -1; - // FIXME: IOS doesn't clear the content access map here. - m_ContentAccessMap.clear(); - m_AccessIdentID = 0; - INFO_LOG(IOS_ES, "ES: Close"); m_is_active = false; // clear the NAND content cache to make sure nothing remains open. @@ -441,16 +414,18 @@ IPCCommandResult ES::IOCtlV(const IOCtlVRequest& request) return ImportTitleCancel(*context, request); case IOCTL_ES_GETDEVICEID: return GetDeviceId(request); - case IOCTL_ES_OPENTITLECONTENT: - return OpenTitleContent(context->uid, request); + case IOCTL_ES_OPENCONTENT: return OpenContent(context->uid, request); + case IOCTL_ES_OPEN_ACTIVE_TITLE_CONTENT: + return OpenActiveTitleContent(context->uid, request); case IOCTL_ES_READCONTENT: return ReadContent(context->uid, request); case IOCTL_ES_CLOSECONTENT: return CloseContent(context->uid, request); case IOCTL_ES_SEEKCONTENT: return SeekContent(context->uid, request); + case IOCTL_ES_GETTITLEDIR: return GetTitleDirectory(request); case IOCTL_ES_GETTITLEID: diff --git a/Source/Core/Core/IOS/ES/ES.h b/Source/Core/Core/IOS/ES/ES.h index 747fa0a942..f87e7cf50a 100644 --- a/Source/Core/Core/IOS/ES/ES.h +++ b/Source/Core/Core/IOS/ES/ES.h @@ -57,9 +57,11 @@ public: struct OpenedContent { - u64 m_title_id; + bool m_opened = false; + u64 m_title_id = 0; IOS::ES::Content m_content; - u32 m_position; + u32 m_position = 0; + u32 m_uid = 0; }; struct TitleImportContext @@ -152,7 +154,7 @@ private: IOCTL_ES_ADDTITLEFINISH = 0x06, IOCTL_ES_GETDEVICEID = 0x07, IOCTL_ES_LAUNCH = 0x08, - IOCTL_ES_OPENCONTENT = 0x09, + IOCTL_ES_OPEN_ACTIVE_TITLE_CONTENT = 0x09, IOCTL_ES_READCONTENT = 0x0A, IOCTL_ES_CLOSECONTENT = 0x0B, IOCTL_ES_GETOWNEDTITLECNT = 0x0C, @@ -179,7 +181,7 @@ private: IOCTL_ES_SETUID = 0x21, IOCTL_ES_DELETETITLECONTENT = 0x22, IOCTL_ES_SEEKCONTENT = 0x23, - IOCTL_ES_OPENTITLECONTENT = 0x24, + IOCTL_ES_OPENCONTENT = 0x24, IOCTL_ES_LAUNCHBC = 0x25, IOCTL_ES_EXPORTTITLEINIT = 0x26, IOCTL_ES_EXPORTCONTENTBEGIN = 0x27, @@ -258,7 +260,7 @@ private: IPCCommandResult DeleteStreamKey(const IOCtlVRequest& request); // Title contents - IPCCommandResult OpenTitleContent(u32 uid, const IOCtlVRequest& request); + IPCCommandResult OpenActiveTitleContent(u32 uid, const IOCtlVRequest& request); IPCCommandResult OpenContent(u32 uid, const IOCtlVRequest& request); IPCCommandResult ReadContent(u32 uid, const IOCtlVRequest& request); IPCCommandResult CloseContent(u32 uid, const IOCtlVRequest& request); @@ -340,12 +342,10 @@ private: static const DiscIO::NANDContentLoader& AccessContentDevice(u64 title_id); - u32 OpenTitleContent(u32 CFD, u64 TitleID, u16 Index); + s32 OpenContent(const IOS::ES::TMDReader& tmd, u16 content_index, u32 uid); - using ContentAccessMap = std::map; - ContentAccessMap m_ContentAccessMap; - - u32 m_AccessIdentID = 0; + using ContentTable = std::array; + ContentTable m_content_table; ContextArray m_contexts; }; diff --git a/Source/Core/Core/IOS/ES/TitleContents.cpp b/Source/Core/Core/IOS/ES/TitleContents.cpp index b825214a7e..33dbd6d038 100644 --- a/Source/Core/Core/IOS/ES/TitleContents.cpp +++ b/Source/Core/Core/IOS/ES/TitleContents.cpp @@ -20,103 +20,114 @@ namespace HLE { namespace Device { -u32 ES::OpenTitleContent(u32 CFD, u64 TitleID, u16 Index) +s32 ES::OpenContent(const IOS::ES::TMDReader& tmd, u16 content_index, u32 uid) { - const DiscIO::NANDContentLoader& Loader = AccessContentDevice(TitleID); + const u64 title_id = tmd.GetTitleId(); + const DiscIO::NANDContentLoader& loader = AccessContentDevice(title_id); - if (!Loader.IsValid() || !Loader.GetTMD().IsValid() || !Loader.GetTicket().IsValid()) + if (!loader.IsValid()) + return FS_ENOENT; + + const DiscIO::NANDContent* content = loader.GetContentByIndex(content_index); + if (!content) + return FS_ENOENT; + + for (size_t i = 0; i < m_content_table.size(); ++i) { - WARN_LOG(IOS_ES, "ES: loader not valid for %" PRIx64, TitleID); - return 0xffffffff; + OpenedContent& entry = m_content_table[i]; + if (entry.m_opened) + continue; + + entry.m_opened = true; + entry.m_position = 0; + entry.m_content = content->m_metadata; + entry.m_title_id = title_id; + entry.m_uid = uid; + INFO_LOG(IOS_ES, "OpenContent: title ID %016" PRIx64 ", UID 0x%x, CFD %zu", title_id, uid, i); + return static_cast(i); } - const DiscIO::NANDContent* pContent = Loader.GetContentByIndex(Index); - - if (pContent == nullptr) - { - return 0xffffffff; // TODO: what is the correct error value here? - } - - OpenedContent content; - content.m_position = 0; - content.m_content = pContent->m_metadata; - content.m_title_id = TitleID; - - pContent->m_Data->Open(); - - m_ContentAccessMap[CFD] = content; - return CFD; -} - -IPCCommandResult ES::OpenTitleContent(u32 uid, const IOCtlVRequest& request) -{ - if (!request.HasNumberOfValidVectors(3, 0)) - return GetDefaultReply(ES_EINVAL); - - u64 TitleID = Memory::Read_U64(request.in_vectors[0].address); - u32 Index = Memory::Read_U32(request.in_vectors[2].address); - - s32 CFD = OpenTitleContent(m_AccessIdentID++, TitleID, Index); - - INFO_LOG(IOS_ES, "IOCTL_ES_OPENTITLECONTENT: TitleID: %016" PRIx64 " Index %i -> got CFD %x", - TitleID, Index, CFD); - - return GetDefaultReply(CFD); + return FS_EFDEXHAUSTED; } IPCCommandResult ES::OpenContent(u32 uid, const IOCtlVRequest& request) { - if (!request.HasNumberOfValidVectors(1, 0)) + if (!request.HasNumberOfValidVectors(3, 0) || request.in_vectors[0].size != sizeof(u64) || + request.in_vectors[1].size != sizeof(IOS::ES::TicketView) || + request.in_vectors[2].size != sizeof(u32)) + { return GetDefaultReply(ES_EINVAL); - u32 Index = Memory::Read_U32(request.in_vectors[0].address); + } + + const u64 title_id = Memory::Read_U64(request.in_vectors[0].address); + const u32 content_index = Memory::Read_U32(request.in_vectors[2].address); + // TODO: check the ticket view, check permissions. + + const auto tmd = FindInstalledTMD(title_id); + if (!tmd.IsValid()) + return GetDefaultReply(FS_ENOENT); + + return GetDefaultReply(OpenContent(tmd, content_index, uid)); +} + +IPCCommandResult ES::OpenActiveTitleContent(u32 caller_uid, const IOCtlVRequest& request) +{ + if (!request.HasNumberOfValidVectors(1, 0) || request.in_vectors[0].size != sizeof(u32)) + return GetDefaultReply(ES_EINVAL); + + const u32 content_index = Memory::Read_U32(request.in_vectors[0].address); if (!GetTitleContext().active) return GetDefaultReply(ES_EINVAL); - s32 CFD = OpenTitleContent(m_AccessIdentID++, GetTitleContext().tmd.GetTitleId(), Index); - INFO_LOG(IOS_ES, "IOCTL_ES_OPENCONTENT: Index %i -> got CFD %x", Index, CFD); + IOS::ES::UIDSys uid_map{Common::FROM_SESSION_ROOT}; + const u32 uid = uid_map.GetOrInsertUIDForTitle(GetTitleContext().tmd.GetTitleId()); + if (caller_uid != 0 && caller_uid != uid) + return GetDefaultReply(ES_EACCES); - return GetDefaultReply(CFD); + return GetDefaultReply(OpenContent(GetTitleContext().tmd, content_index, caller_uid)); } IPCCommandResult ES::ReadContent(u32 uid, const IOCtlVRequest& request) { - if (!request.HasNumberOfValidVectors(1, 1)) + if (!request.HasNumberOfValidVectors(1, 1) || request.in_vectors[0].size != sizeof(u32)) return GetDefaultReply(ES_EINVAL); - u32 CFD = Memory::Read_U32(request.in_vectors[0].address); - u32 Size = request.io_vectors[0].size; - u32 Addr = request.io_vectors[0].address; + const u32 cfd = Memory::Read_U32(request.in_vectors[0].address); + u32 size = request.io_vectors[0].size; + const u32 addr = request.io_vectors[0].address; - auto itr = m_ContentAccessMap.find(CFD); - if (itr == m_ContentAccessMap.end()) + if (cfd >= m_content_table.size()) + return GetDefaultReply(ES_EINVAL); + OpenedContent& entry = m_content_table[cfd]; + + if (entry.m_uid != uid) + return GetDefaultReply(ES_EACCES); + if (!entry.m_opened) + return GetDefaultReply(IPC_EINVAL); + + // XXX: make this reuse the FS code... ES just does a simple "IOS_Read" call here + // instead of all this duplicated filesystem logic. + + if (entry.m_position + size > entry.m_content.size) + size = static_cast(entry.m_content.size) - entry.m_position; + + if (size > 0) { - return GetDefaultReply(-1); - } - OpenedContent& rContent = itr->second; - - u8* pDest = Memory::GetPointer(Addr); - - if (rContent.m_position + Size > rContent.m_content.size) - { - Size = static_cast(rContent.m_content.size) - rContent.m_position; - } - - if (Size > 0) - { - if (pDest) + if (addr) { - const DiscIO::NANDContentLoader& ContentLoader = AccessContentDevice(rContent.m_title_id); + const DiscIO::NANDContentLoader& ContentLoader = AccessContentDevice(entry.m_title_id); // ContentLoader should never be invalid; rContent has been created by it. if (ContentLoader.IsValid() && ContentLoader.GetTicket().IsValid()) { const DiscIO::NANDContent* pContent = - ContentLoader.GetContentByIndex(rContent.m_content.index); - if (!pContent->m_Data->GetRange(rContent.m_position, Size, pDest)) - ERROR_LOG(IOS_ES, "ES: failed to read %u bytes from %u!", Size, rContent.m_position); + ContentLoader.GetContentByIndex(entry.m_content.index); + pContent->m_Data->Open(); + if (!pContent->m_Data->GetRange(entry.m_position, size, Memory::GetPointer(addr))) + ERROR_LOG(IOS_ES, "ES: failed to read %u bytes from %u!", size, entry.m_position); } - rContent.m_position += Size; + entry.m_position += size; } else { @@ -124,39 +135,35 @@ IPCCommandResult ES::ReadContent(u32 uid, const IOCtlVRequest& request) } } - DEBUG_LOG(IOS_ES, - "IOCTL_ES_READCONTENT: CFD %x, Address 0x%x, Size %i -> stream pos %i (Index %i)", CFD, - Addr, Size, rContent.m_position, rContent.m_content.index); - - return GetDefaultReply(Size); + return GetDefaultReply(size); } IPCCommandResult ES::CloseContent(u32 uid, const IOCtlVRequest& request) { - if (!request.HasNumberOfValidVectors(1, 0)) + if (!request.HasNumberOfValidVectors(1, 0) || request.in_vectors[0].size != sizeof(u32)) return GetDefaultReply(ES_EINVAL); - u32 CFD = Memory::Read_U32(request.in_vectors[0].address); + const u32 cfd = Memory::Read_U32(request.in_vectors[0].address); + if (cfd >= m_content_table.size()) + return GetDefaultReply(ES_EINVAL); - INFO_LOG(IOS_ES, "IOCTL_ES_CLOSECONTENT: CFD %x", CFD); + OpenedContent& entry = m_content_table[cfd]; + if (entry.m_uid != uid) + return GetDefaultReply(ES_EACCES); + if (!entry.m_opened) + return GetDefaultReply(IPC_EINVAL); - auto itr = m_ContentAccessMap.find(CFD); - if (itr == m_ContentAccessMap.end()) - { - return GetDefaultReply(-1); - } - - const DiscIO::NANDContentLoader& ContentLoader = AccessContentDevice(itr->second.m_title_id); + // XXX: again, this should be a simple IOS_Close. + const DiscIO::NANDContentLoader& ContentLoader = AccessContentDevice(entry.m_title_id); // ContentLoader should never be invalid; we shouldn't be here if ES_OPENCONTENT failed before. if (ContentLoader.IsValid()) { - const DiscIO::NANDContent* pContent = - ContentLoader.GetContentByIndex(itr->second.m_content.index); - pContent->m_Data->Close(); + const DiscIO::NANDContent* content = ContentLoader.GetContentByIndex(entry.m_content.index); + content->m_Data->Close(); } - m_ContentAccessMap.erase(itr); - + entry = {}; + INFO_LOG(IOS_ES, "CloseContent: CFD %u", cfd); return GetDefaultReply(IPC_SUCCESS); } @@ -165,36 +172,36 @@ IPCCommandResult ES::SeekContent(u32 uid, const IOCtlVRequest& request) if (!request.HasNumberOfValidVectors(3, 0)) return GetDefaultReply(ES_EINVAL); - u32 CFD = Memory::Read_U32(request.in_vectors[0].address); + const u32 cfd = Memory::Read_U32(request.in_vectors[0].address); + if (cfd >= m_content_table.size()) + return GetDefaultReply(ES_EINVAL); + + OpenedContent& entry = m_content_table[cfd]; + if (entry.m_uid != uid) + return GetDefaultReply(ES_EACCES); + if (!entry.m_opened) + return GetDefaultReply(IPC_EINVAL); + u32 Addr = Memory::Read_U32(request.in_vectors[1].address); u32 Mode = Memory::Read_U32(request.in_vectors[2].address); - auto itr = m_ContentAccessMap.find(CFD); - if (itr == m_ContentAccessMap.end()) - { - return GetDefaultReply(-1); - } - OpenedContent& rContent = itr->second; - + // XXX: This should be a simple IOS_Seek. switch (Mode) { case 0: // SET - rContent.m_position = Addr; + entry.m_position = Addr; break; case 1: // CUR - rContent.m_position += Addr; + entry.m_position += Addr; break; case 2: // END - rContent.m_position = static_cast(rContent.m_content.size) + Addr; + entry.m_position = static_cast(entry.m_content.size) + Addr; break; } - DEBUG_LOG(IOS_ES, "IOCTL_ES_SEEKCONTENT: CFD %x, Address 0x%x, Mode %i -> Pos %i", CFD, Addr, - Mode, rContent.m_position); - - return GetDefaultReply(rContent.m_position); + return GetDefaultReply(entry.m_position); } } // namespace Device } // namespace HLE diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index 92b7abb6ea..bc2a0f20aa 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -73,7 +73,7 @@ static Common::Event g_compressAndDumpStateSyncEvent; static std::thread g_save_thread; // Don't forget to increase this after doing changes on the savestate system -static const u32 STATE_VERSION = 86; // Last changed in PR 2353 +static const u32 STATE_VERSION = 87; // Last changed in PR 5707 // Maps savestate versions to Dolphin versions. // Versions after 42 don't need to be added to this list,