From b6d3df333588f14ad3a56b10fb6222cb2b9a54fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Fri, 8 Mar 2019 16:25:33 +0100 Subject: [PATCH] fs_mitm: Fix mismatched new[] / delete (#389) * fs.mitm: Fix mismatched new[] / delete Using delete instead of delete[] on a pointer given by new[] is undefined behaviour. For memory sources, malloc/free are used because cleaning up is tricky when data can be either allocated with new (RomfsHeader) or new[] (metadata). * set.mitm: Fix mismatched new[] / delete --- .../source/fs_mitm/fsmitm_romfsbuild.cpp | 24 +++++++++---------- .../source/fs_mitm/fsmitm_romfsbuild.hpp | 5 ++-- .../source/set_mitm/setsys_settings_items.cpp | 2 +- 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.cpp b/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.cpp index 611b949da..b2073083d 100644 --- a/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.cpp +++ b/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.cpp @@ -49,7 +49,7 @@ void RomFSBuildContext::VisitDirectory(FsFileSystem *filesys, RomFSBuildDirector strcat(child->path + parent->path_len, this->dir_entry.name); if (!this->AddDirectory(parent, child, NULL)) { - delete child->path; + delete[] child->path; delete child; } else { child_dirs.push_back(child); @@ -72,7 +72,7 @@ void RomFSBuildContext::VisitDirectory(FsFileSystem *filesys, RomFSBuildDirector child->size = this->dir_entry.fileSize; if (!this->AddFile(parent, child)) { - delete child->path; + delete[] child->path; delete child; } } else { @@ -125,7 +125,7 @@ void RomFSBuildContext::VisitDirectory(RomFSBuildDirectoryContext *parent, u32 p child->source = this->cur_source_type; child->orig_offset = cur_file->offset; if (!this->AddFile(parent, child)) { - delete child->path; + delete[] child->path; delete child; } if (cur_file->sibling == ROMFS_ENTRY_EMPTY) { @@ -153,7 +153,7 @@ void RomFSBuildContext::VisitDirectory(RomFSBuildDirectoryContext *parent, u32 p RomFSBuildDirectoryContext *real = NULL; if (!this->AddDirectory(parent, child, &real)) { - delete child->path; + delete[] child->path; delete child; } if (real == NULL) { @@ -246,8 +246,10 @@ void RomFSBuildContext::Build(std::vector *out_infos) { this->file_hash_table_size = 4 * file_hash_table_entry_count; /* Assign metadata pointers */ - RomFSHeader *header = new RomFSHeader({0}); - u8 *metadata = new u8[this->dir_hash_table_size + this->dir_table_size + this->file_hash_table_size + this->file_table_size]; + auto *header = reinterpret_cast(std::malloc(sizeof(RomFSHeader))); + *header = {}; + const size_t metadata_size = this->dir_hash_table_size + this->dir_table_size + this->file_hash_table_size + this->file_table_size; + u8 *metadata = reinterpret_cast(std::malloc(metadata_size)); u32 *dir_hash_table = (u32 *)((uintptr_t)metadata); RomFSDirectoryEntry *dir_table = (RomFSDirectoryEntry *)((uintptr_t)dir_hash_table + this->dir_hash_table_size); u32 *file_hash_table = (u32 *)((uintptr_t)dir_table + this->dir_table_size); @@ -372,7 +374,7 @@ void RomFSBuildContext::Build(std::vector *out_infos) { /* Delete directories. */ for (const auto &it : this->directories) { cur_dir = it.second; - delete cur_dir->path; + delete[] cur_dir->path; delete cur_dir; } this->root = NULL; @@ -381,7 +383,7 @@ void RomFSBuildContext::Build(std::vector *out_infos) { /* Delete files. */ for (const auto &it : this->files) { cur_file = it.second; - delete cur_file->path; + delete[] cur_file->path; delete cur_file; } this->files.clear(); @@ -397,13 +399,11 @@ void RomFSBuildContext::Build(std::vector *out_infos) { header->dir_table_ofs = header->dir_hash_table_ofs + header->dir_hash_table_size; header->file_hash_table_ofs = header->dir_table_ofs + header->dir_table_size; header->file_table_ofs = header->file_hash_table_ofs + header->file_hash_table_size; - - const size_t metadata_size = this->dir_hash_table_size + this->dir_table_size + this->file_hash_table_size + this->file_table_size; - + /* Try to save metadata to the SD card, to save on memory space. */ if (R_SUCCEEDED(Utils::SaveSdFileForAtmosphere(this->title_id, ROMFS_METADATA_FILE_PATH, metadata, metadata_size))) { out_infos->emplace_back(header->dir_hash_table_ofs, metadata_size, RomFSDataSource::MetaData); - delete metadata; + std::free(metadata); } else { out_infos->emplace_back(header->dir_hash_table_ofs, metadata_size, metadata, RomFSDataSource::Memory); } diff --git a/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.hpp b/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.hpp index e76b7e7c0..bdc5f0058 100644 --- a/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.hpp +++ b/stratosphere/ams_mitm/source/fs_mitm/fsmitm_romfsbuild.hpp @@ -15,6 +15,7 @@ */ #pragma once +#include #include #include @@ -120,10 +121,10 @@ struct RomFSSourceInfo { case RomFSDataSource::MetaData: break; case RomFSDataSource::LooseFile: - delete this->loose_source_info.path; + delete[] this->loose_source_info.path; break; case RomFSDataSource::Memory: - delete this->memory_source_info.data; + std::free((void*)this->memory_source_info.data); break; default: fatalSimple(0xF601); diff --git a/stratosphere/ams_mitm/source/set_mitm/setsys_settings_items.cpp b/stratosphere/ams_mitm/source/set_mitm/setsys_settings_items.cpp index cd845c492..3df268b14 100644 --- a/stratosphere/ams_mitm/source/set_mitm/setsys_settings_items.cpp +++ b/stratosphere/ams_mitm/source/set_mitm/setsys_settings_items.cpp @@ -257,7 +257,7 @@ void SettingsItemManager::LoadConfiguration() { char *config_buf = new char[0x10000]; std::memset(config_buf, 0, 0x10000); ON_SCOPE_EXIT { - delete config_buf; + delete[] config_buf; }; /* Read from file. */