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
This commit is contained in:
MrPurple666 2025-05-16 01:44:57 -03:00
parent 1ffa98a40d
commit 0929bfc156
2 changed files with 37 additions and 3 deletions

View file

@ -18,6 +18,10 @@ public:
} }
std::pair<void*, size_t> FreeBlock(void* block_ptr, size_t size) { std::pair<void*, size_t> FreeBlock(void* block_ptr, size_t size) {
if (block_ptr == nullptr || size == 0) {
return {nullptr, 0};
}
std::scoped_lock lk(m_mutex); std::scoped_lock lk(m_mutex);
// Check to see if we are adjacent to any regions. // Check to see if we are adjacent to any regions.
@ -41,6 +45,11 @@ public:
} }
void AllocateBlock(void* block_ptr, size_t size) { 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); std::scoped_lock lk(m_mutex);
auto address = reinterpret_cast<uintptr_t>(block_ptr); auto address = reinterpret_cast<uintptr_t>(block_ptr);

View file

@ -491,6 +491,12 @@ public:
// Intersect the range with our address space. // Intersect the range with our address space.
AdjustMap(&virtual_offset, &length); 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. // We are removing a placeholder.
free_manager.AllocateBlock(virtual_base + virtual_offset, length); free_manager.AllocateBlock(virtual_base + virtual_offset, length);
@ -520,13 +526,21 @@ public:
// Intersect the range with our address space. // Intersect the range with our address space.
AdjustMap(&virtual_offset, &length); AdjustMap(&virtual_offset, &length);
// Skip if length is zero after adjustment
if (length == 0) {
return;
}
// Merge with any adjacent placeholder mappings. // Merge with any adjacent placeholder mappings.
auto [merged_pointer, merged_size] = auto [merged_pointer, merged_size] =
free_manager.FreeBlock(virtual_base + virtual_offset, length); free_manager.FreeBlock(virtual_base + virtual_offset, length);
void* ret = mmap(merged_pointer, merged_size, PROT_NONE, // Only attempt to mmap if we have a valid pointer and size
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); if (merged_pointer != nullptr && merged_size > 0) {
ASSERT_MSG(ret != MAP_FAILED, "mmap failed: {}", strerror(errno)); 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) { void Protect(size_t virtual_offset, size_t length, bool read, bool write, bool execute) {
@ -576,19 +590,26 @@ public:
private: private:
/// Release all resources in the object /// Release all resources in the object
void Release() { 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) { if (virtual_map_base != MAP_FAILED) {
int ret = munmap(virtual_map_base, virtual_size); int ret = munmap(virtual_map_base, virtual_size);
ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno)); ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno));
virtual_map_base = reinterpret_cast<u8*>(MAP_FAILED);
} }
if (backing_base != MAP_FAILED) { if (backing_base != MAP_FAILED) {
int ret = munmap(backing_base, backing_size); int ret = munmap(backing_base, backing_size);
ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno)); ASSERT_MSG(ret == 0, "munmap failed: {}", strerror(errno));
backing_base = reinterpret_cast<u8*>(MAP_FAILED);
} }
if (fd != -1) { if (fd != -1) {
int ret = close(fd); int ret = close(fd);
ASSERT_MSG(ret == 0, "close failed: {}", strerror(errno)); 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(virtual_offset + length <= virtual_size);
ASSERT(host_offset + length <= backing_size); ASSERT(host_offset + length <= backing_size);
if (length == 0 || !virtual_base || !impl) { if (length == 0 || !virtual_base || !impl) {
LOG_ERROR(HW_Memory, "Invalid mapping operation: virtual_base or impl is null");
return; 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); 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(length % PageAlignment == 0);
ASSERT(virtual_offset + length <= virtual_size); ASSERT(virtual_offset + length <= virtual_size);
if (length == 0 || !virtual_base || !impl) { if (length == 0 || !virtual_base || !impl) {
LOG_ERROR(HW_Memory, "Invalid unmapping operation: virtual_base or impl is null");
return; return;
} }
LOG_INFO(HW_Memory, "Unmapping memory: virtual_offset={}, length={}", virtual_offset, length);
impl->Unmap(virtual_offset + virtual_base_offset, length); impl->Unmap(virtual_offset + virtual_base_offset, length);
} }