From 0929bfc1560fb42f11f7b93c13633ff87eec0861 Mon Sep 17 00:00:00 2001 From: MrPurple666 Date: Fri, 16 May 2025 01:44:57 -0300 Subject: [PATCH] try fix invalid chunk state when deallocating Enhance memory safety: add null/zero-length checks and pointer resets, improve validation for memory mapping and cleanup on destruction --- src/common/free_region_manager.h | 9 +++++++++ src/common/host_memory.cpp | 31 ++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/common/free_region_manager.h b/src/common/free_region_manager.h index 2e590d6094..e282a35cca 100644 --- a/src/common/free_region_manager.h +++ b/src/common/free_region_manager.h @@ -18,6 +18,10 @@ public: } std::pair FreeBlock(void* block_ptr, size_t size) { + if (block_ptr == nullptr || size == 0) { + return {nullptr, 0}; + } + std::scoped_lock lk(m_mutex); // Check to see if we are adjacent to any regions. @@ -41,6 +45,11 @@ public: } void AllocateBlock(void* block_ptr, size_t size) { + // Skip if pointer is null or size is zero + if (block_ptr == nullptr || size == 0) { + return; + } + std::scoped_lock lk(m_mutex); auto address = reinterpret_cast(block_ptr); diff --git a/src/common/host_memory.cpp b/src/common/host_memory.cpp index e0b5a6a67c..3eee0b66d5 100644 --- a/src/common/host_memory.cpp +++ b/src/common/host_memory.cpp @@ -491,6 +491,12 @@ public: // Intersect the range with our address space. AdjustMap(&virtual_offset, &length); + // Skip if length is zero after adjustment + if (length == 0) { + LOG_DEBUG(HW_Memory, "Skipping zero-length mapping at virtual_offset={}", virtual_offset); + return; + } + // We are removing a placeholder. free_manager.AllocateBlock(virtual_base + virtual_offset, length); @@ -520,13 +526,21 @@ public: // Intersect the range with our address space. AdjustMap(&virtual_offset, &length); + // Skip if length is zero after adjustment + if (length == 0) { + return; + } + // Merge with any adjacent placeholder mappings. auto [merged_pointer, merged_size] = free_manager.FreeBlock(virtual_base + virtual_offset, length); - void* ret = mmap(merged_pointer, merged_size, PROT_NONE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); - ASSERT_MSG(ret != MAP_FAILED, "mmap failed: {}", strerror(errno)); + // Only attempt to mmap if we have a valid pointer and size + if (merged_pointer != nullptr && merged_size > 0) { + void* ret = mmap(merged_pointer, merged_size, PROT_NONE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + ASSERT_MSG(ret != MAP_FAILED, "mmap failed: {}", strerror(errno)); + } } void Protect(size_t virtual_offset, size_t length, bool read, bool write, bool execute) { @@ -576,19 +590,26 @@ public: private: /// Release all resources in the object void Release() { + // Make sure we release resources in the correct order + // First clear the free region manager to avoid any dangling references + free_manager = {}; + if (virtual_map_base != MAP_FAILED) { int ret = munmap(virtual_map_base, virtual_size); ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno)); + virtual_map_base = reinterpret_cast(MAP_FAILED); } if (backing_base != MAP_FAILED) { int ret = munmap(backing_base, backing_size); ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno)); + backing_base = reinterpret_cast(MAP_FAILED); } if (fd != -1) { int ret = close(fd); ASSERT_MSG(ret == 0, "close failed: {}", strerror(errno)); + fd = -1; } } @@ -686,8 +707,10 @@ void HostMemory::Map(size_t virtual_offset, size_t host_offset, size_t length, ASSERT(virtual_offset + length <= virtual_size); ASSERT(host_offset + length <= backing_size); if (length == 0 || !virtual_base || !impl) { + LOG_ERROR(HW_Memory, "Invalid mapping operation: virtual_base or impl is null"); return; } + LOG_INFO(HW_Memory, "Mapping memory: virtual_offset={}, host_offset={}, length={}", virtual_offset, host_offset, length); impl->Map(virtual_offset + virtual_base_offset, host_offset, length, perms); } @@ -696,8 +719,10 @@ void HostMemory::Unmap(size_t virtual_offset, size_t length, bool separate_heap) ASSERT(length % PageAlignment == 0); ASSERT(virtual_offset + length <= virtual_size); if (length == 0 || !virtual_base || !impl) { + LOG_ERROR(HW_Memory, "Invalid unmapping operation: virtual_base or impl is null"); return; } + LOG_INFO(HW_Memory, "Unmapping memory: virtual_offset={}, length={}", virtual_offset, length); impl->Unmap(virtual_offset + virtual_base_offset, length); }