From 92a98a8b19ab8b6f9eefe3b2e8e80f957444d65e Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 13:45:29 -0400 Subject: [PATCH 1/6] xts_archive: Amend initializer order of NAX's constructor Orders the initializer list in the same order the members would be initialized. Avoids compiler warnings. --- src/core/file_sys/xts_archive.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/file_sys/xts_archive.cpp b/src/core/file_sys/xts_archive.cpp index 0173f71c1e..55257da2e3 100644 --- a/src/core/file_sys/xts_archive.cpp +++ b/src/core/file_sys/xts_archive.cpp @@ -45,7 +45,7 @@ static bool CalculateHMAC256(Destination* out, const SourceKey* key, std::size_t return true; } -NAX::NAX(VirtualFile file_) : file(std::move(file_)), header(std::make_unique()) { +NAX::NAX(VirtualFile file_) : header(std::make_unique()), file(std::move(file_)) { std::string path = FileUtil::SanitizePath(file->GetFullPath()); static const std::regex nax_path_regex("/registered/(000000[0-9A-F]{2})/([0-9A-F]{32})\\.nca", std::regex_constants::ECMAScript | @@ -65,7 +65,7 @@ NAX::NAX(VirtualFile file_) : file(std::move(file_)), header(std::make_unique nca_id) - : file(std::move(file_)), header(std::make_unique()) { + : header(std::make_unique()), file(std::move(file_)) { Core::Crypto::SHA256Hash hash{}; mbedtls_sha256(nca_id.data(), nca_id.size(), hash.data(), 0); status = Parse(fmt::format("/registered/000000{:02X}/{}.nca", hash[0], From f272261c210877fadc486d6a0413f4770f04ca89 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 13:46:32 -0400 Subject: [PATCH 2/6] xts_archive: Ensure NAX's type member is always initialized Ensures that the member always has a deterministic value. --- src/core/file_sys/xts_archive.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/file_sys/xts_archive.h b/src/core/file_sys/xts_archive.h index 55d2154a6d..0dd0536f18 100644 --- a/src/core/file_sys/xts_archive.h +++ b/src/core/file_sys/xts_archive.h @@ -60,7 +60,7 @@ private: VirtualFile file; Loader::ResultStatus status; - NAXContentType type; + NAXContentType type{}; VirtualFile dec_file; From c8c410565927b54b2a1353ca00a1c4de38a74816 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 13:54:43 -0400 Subject: [PATCH 3/6] nax: Avoid unnecessary calls to AsNCA() in IdentifyType() AsNCA() allocates an NCA instance every time it's called. In the current manner it's used, it's quite inefficient as it's making a redundant allocation. We can just amend the order of the conditionals to make it easier to just call it once. --- src/core/loader/nax.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/core/loader/nax.cpp b/src/core/loader/nax.cpp index b46d81c022..02a0d5ba74 100644 --- a/src/core/loader/nax.cpp +++ b/src/core/loader/nax.cpp @@ -21,12 +21,16 @@ AppLoader_NAX::~AppLoader_NAX() = default; FileType AppLoader_NAX::IdentifyType(const FileSys::VirtualFile& file) { FileSys::NAX nax(file); - if (nax.GetStatus() == ResultStatus::Success && nax.AsNCA() != nullptr && - nax.AsNCA()->GetStatus() == ResultStatus::Success) { - return FileType::NAX; + if (nax.GetStatus() != ResultStatus::Success) { + return FileType::Error; } - return FileType::Error; + const auto nca = nax.AsNCA(); + if (nca == nullptr || nca->GetStatus() != ResultStatus::Success) { + return FileType::Error; + } + + return FileType::NAX; } ResultStatus AppLoader_NAX::Load(Kernel::SharedPtr& process) { From 45195a51a76b3000e028234f619a4d15bff443eb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 14:13:00 -0400 Subject: [PATCH 4/6] nax: Avoid re-parsing NAX data with GetFileType() An instance of the NAX apploader already has an existing NAX instance in memory. Calling directly into IdentifyType() directly would re-parse the whole file again into yet another NAX instance, only to toss it away again. This gets rid of unnecessary/redundant file parsing and allocations. --- src/core/loader/nax.cpp | 28 ++++++++++++++++++---------- src/core/loader/nax.h | 4 +--- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/core/loader/nax.cpp b/src/core/loader/nax.cpp index 02a0d5ba74..5d4380684f 100644 --- a/src/core/loader/nax.cpp +++ b/src/core/loader/nax.cpp @@ -11,16 +11,8 @@ #include "core/loader/nca.h" namespace Loader { - -AppLoader_NAX::AppLoader_NAX(FileSys::VirtualFile file) - : AppLoader(file), nax(std::make_unique(file)), - nca_loader(std::make_unique(nax->GetDecrypted())) {} - -AppLoader_NAX::~AppLoader_NAX() = default; - -FileType AppLoader_NAX::IdentifyType(const FileSys::VirtualFile& file) { - FileSys::NAX nax(file); - +namespace { +FileType IdentifyTypeImpl(const FileSys::NAX& nax) { if (nax.GetStatus() != ResultStatus::Success) { return FileType::Error; } @@ -32,6 +24,22 @@ FileType AppLoader_NAX::IdentifyType(const FileSys::VirtualFile& file) { return FileType::NAX; } +} // Anonymous namespace + +AppLoader_NAX::AppLoader_NAX(FileSys::VirtualFile file) + : AppLoader(file), nax(std::make_unique(file)), + nca_loader(std::make_unique(nax->GetDecrypted())) {} + +AppLoader_NAX::~AppLoader_NAX() = default; + +FileType AppLoader_NAX::IdentifyType(const FileSys::VirtualFile& file) { + const FileSys::NAX nax(file); + return IdentifyTypeImpl(nax); +} + +FileType AppLoader_NAX::GetFileType() { + return IdentifyTypeImpl(*nax); +} ResultStatus AppLoader_NAX::Load(Kernel::SharedPtr& process) { if (is_loaded) { diff --git a/src/core/loader/nax.h b/src/core/loader/nax.h index 4dbae29182..56605fe45c 100644 --- a/src/core/loader/nax.h +++ b/src/core/loader/nax.h @@ -31,9 +31,7 @@ public: */ static FileType IdentifyType(const FileSys::VirtualFile& file); - FileType GetFileType() override { - return IdentifyType(file); - } + FileType GetFileType() override; ResultStatus Load(Kernel::SharedPtr& process) override; From 2752183883604673e5058a3a6f62defcdca49a72 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 14:16:22 -0400 Subject: [PATCH 5/6] xts_archive: Make AsNCA() return a std::unique_ptr instead of a std::shared_ptr std::shared_ptr isn't strictly necessary here and is only ever used in contexts where the object doesn't depend on being shared. This also makes the interface more flexible, as it's possible to create a std::shared_ptr from a std::unique_ptr (std::shared_ptr has a constructor that accepts a std::unique_ptr), but not the other way around. --- src/core/file_sys/xts_archive.cpp | 4 ++-- src/core/file_sys/xts_archive.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/file_sys/xts_archive.cpp b/src/core/file_sys/xts_archive.cpp index 55257da2e3..6935d2fa22 100644 --- a/src/core/file_sys/xts_archive.cpp +++ b/src/core/file_sys/xts_archive.cpp @@ -138,9 +138,9 @@ VirtualFile NAX::GetDecrypted() const { return dec_file; } -std::shared_ptr NAX::AsNCA() const { +std::unique_ptr NAX::AsNCA() const { if (type == NAXContentType::NCA) - return std::make_shared(GetDecrypted()); + return std::make_unique(GetDecrypted()); return nullptr; } diff --git a/src/core/file_sys/xts_archive.h b/src/core/file_sys/xts_archive.h index 0dd0536f18..6e2fc4d2e0 100644 --- a/src/core/file_sys/xts_archive.h +++ b/src/core/file_sys/xts_archive.h @@ -38,7 +38,7 @@ public: VirtualFile GetDecrypted() const; - std::shared_ptr AsNCA() const; + std::unique_ptr AsNCA() const; NAXContentType GetContentType() const; From 2e5f0e5024fac21808de12fd9008feb85dc2f02c Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 19 Sep 2018 14:23:11 -0400 Subject: [PATCH 6/6] xts_archive: Remove unused variables from CalculateHMAC256() These variables aren't used, which still has an impact, as std::vector cannot be optimized away by the compiler (it's constructor and destructor are both non-trivial), so this was just wasting memory. --- src/core/file_sys/xts_archive.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/file_sys/xts_archive.cpp b/src/core/file_sys/xts_archive.cpp index 6935d2fa22..e937d14039 100644 --- a/src/core/file_sys/xts_archive.cpp +++ b/src/core/file_sys/xts_archive.cpp @@ -30,9 +30,6 @@ static bool CalculateHMAC256(Destination* out, const SourceKey* key, std::size_t mbedtls_md_context_t context; mbedtls_md_init(&context); - const auto key_f = reinterpret_cast(key); - const std::vector key_v(key_f, key_f + key_length); - if (mbedtls_md_setup(&context, mbedtls_md_info_from_type(MBEDTLS_MD_SHA256), 1) || mbedtls_md_hmac_starts(&context, reinterpret_cast(key), key_length) || mbedtls_md_hmac_update(&context, reinterpret_cast(data), data_length) ||