From 42f16950222e5f7559075795a74d0f62f1f01843 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 a832a42fe02edca74f00143f06e682a124de5034 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 20ebe330713e3ff6209d57de9c2565022f14cf62 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 2eeb830edb109856f8db4cc315a7ce3c8f0887e2 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 d2736c4ddcf4bfd8a8218e61147c6e6dbd5797b9 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 a3a51a7b98bb587d75a5a56dacdd66303e93df91 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) ||