diff --git a/src/core/address_space.cpp b/src/core/address_space.cpp index 3c65010e..dec761ba 100644 --- a/src/core/address_space.cpp +++ b/src/core/address_space.cpp @@ -218,24 +218,36 @@ struct AddressSpace::Impl { void Protect(VAddr virtual_addr, size_t size, bool read, bool write, bool execute) { DWORD new_flags{}; + if (read && write && execute) { new_flags = PAGE_EXECUTE_READWRITE; } else if (read && write) { new_flags = PAGE_READWRITE; } else if (read && !write) { new_flags = PAGE_READONLY; - } else if (execute && !read && !write) { + } else if (execute && !read && not write) { new_flags = PAGE_EXECUTE; } else if (!read && !write && !execute) { new_flags = PAGE_NOACCESS; } else { - LOG_CRITICAL(Common_Memory, "Unsupported protection flag combination"); + LOG_CRITICAL(Common_Memory, + "Unsupported protection flag combination for address {:#x}, size {}", + virtual_addr, size); + return; } DWORD old_flags{}; - if (!VirtualProtect(reinterpret_cast(virtual_addr), size, new_flags, &old_flags)) { - LOG_CRITICAL(Common_Memory, "Failed to change virtual memory protection"); + bool success = + VirtualProtect(reinterpret_cast(virtual_addr), size, new_flags, &old_flags); + + if (!success) { + LOG_ERROR(Common_Memory, + "Failed to change virtual memory protection for address {:#x}, size {}", + virtual_addr, size); } + + // Use assert to ensure success in debug builds + assert(success && "Failed to change virtual memory protection"); } HANDLE process{}; @@ -477,10 +489,11 @@ void AddressSpace::Unmap(VAddr virtual_addr, size_t size, VAddr start_in_vma, VA } void AddressSpace::Protect(VAddr virtual_addr, size_t size, MemoryPermission perms) { - bool read = static_cast(perms & MemoryPermission::Read) != 0; - bool write = static_cast(perms & MemoryPermission::Write) != 0; - bool execute = static_cast(perms & MemoryPermission::Execute) != - 0; // Assuming you have an Execute permission + const bool read = True(perms & MemoryPermission::Read); + + const bool write = True(perms & MemoryPermission::Write); + + const bool execute = True(perms & MemoryPermission::Execute); return impl->Protect(virtual_addr, size, read, write, execute); } diff --git a/src/core/libraries/kernel/memory_management.cpp b/src/core/libraries/kernel/memory_management.cpp index 55f37e34..fa3e1bc4 100644 --- a/src/core/libraries/kernel/memory_management.cpp +++ b/src/core/libraries/kernel/memory_management.cpp @@ -209,32 +209,16 @@ int PS4_SYSV_ABI sceKernelQueryMemoryProtection(void* addr, void** start, void** } int PS4_SYSV_ABI sceKernelMProtect(const void* addr, size_t size, int prot) { - Core::MemoryManager* memory_manager = Core::Memory::Instance(); - - // Cast the 'prot' integer to 'MemoryProt' enum type Core::MemoryProt protection_flags = static_cast(prot); - - int result = memory_manager->Protect(std::bit_cast(addr), size, protection_flags); - if (result != ORBIS_OK) { - LOG_ERROR(Kernel_Vmm, "MProtect failed with result {}", result); - } - return result; + return memory_manager->Protect(std::bit_cast(addr), size, protection_flags); } int PS4_SYSV_ABI sceKernelMTypeProtect(const void* addr, size_t size, int mtype, int prot) { - Core::MemoryManager* memory_manager = Core::Memory::Instance(); - - // Cast the 'prot' integer to 'MemoryProt' enum type Core::MemoryProt protection_flags = static_cast(prot); - - int result = memory_manager->MTypeProtect(std::bit_cast(addr), size, - static_cast(mtype), protection_flags); - if (result != ORBIS_OK) { - LOG_ERROR(Kernel_Vmm, "MTypeProtect failed with result {}", result); - } - return result; + return memory_manager->MTypeProtect(std::bit_cast(addr), size, + static_cast(mtype), protection_flags); } int PS4_SYSV_ABI sceKernelDirectMemoryQuery(u64 offset, int flags, OrbisQueryInfo* query_info, diff --git a/src/core/memory.cpp b/src/core/memory.cpp index c06ad832..9280f63b 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -286,12 +286,14 @@ int MemoryManager::Protect(VAddr addr, size_t size, MemoryProt prot) { } // Validate protection flags - MemoryProt valid_flags = MemoryProt::NoAccess | MemoryProt::CpuRead | MemoryProt::CpuReadWrite | - MemoryProt::GpuRead | MemoryProt::GpuWrite | MemoryProt::GpuReadWrite; + constexpr static MemoryProt valid_flags = MemoryProt::NoAccess | MemoryProt::CpuRead | + MemoryProt::CpuReadWrite | MemoryProt::GpuRead | + MemoryProt::GpuWrite | MemoryProt::GpuReadWrite; - if ((prot & ~valid_flags) != MemoryProt::NoAccess) { - LOG_ERROR(Core, "Invalid protection flags, prot: {:#x}, GpuWrite: {:#x}", - static_cast(prot), static_cast(MemoryProt::GpuWrite)); + MemoryProt invalid_flags = prot & ~valid_flags; + if (u32(invalid_flags) != 0 && u32(invalid_flags) != u32(MemoryProt::NoAccess)) { + LOG_ERROR(Core, "Invalid protection flags: prot = {:#x}, invalid flags = {:#x}", u32(prot), + invalid_flags); return ORBIS_KERNEL_ERROR_EINVAL; } @@ -301,19 +303,19 @@ int MemoryManager::Protect(VAddr addr, size_t size, MemoryProt prot) { // Set permissions Core::MemoryPermission perms{}; - if ((prot & MemoryProt::CpuRead) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::CpuRead)) { perms |= Core::MemoryPermission::Read; } - if ((prot & MemoryProt::CpuReadWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::CpuReadWrite)) { perms |= Core::MemoryPermission::ReadWrite; } - if ((prot & MemoryProt::GpuRead) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuRead)) { perms |= Core::MemoryPermission::Read; } - if ((prot & MemoryProt::GpuWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuWrite)) { perms |= Core::MemoryPermission::Write; } - if ((prot & MemoryProt::GpuReadWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuReadWrite)) { perms |= Core::MemoryPermission::ReadWrite; } @@ -340,12 +342,14 @@ int MemoryManager::MTypeProtect(VAddr addr, size_t size, VMAType mtype, MemoryPr } // Validate protection flags - MemoryProt valid_flags = MemoryProt::NoAccess | MemoryProt::CpuRead | MemoryProt::CpuReadWrite | - MemoryProt::GpuRead | MemoryProt::GpuWrite | MemoryProt::GpuReadWrite; + constexpr static MemoryProt valid_flags = MemoryProt::NoAccess | MemoryProt::CpuRead | + MemoryProt::CpuReadWrite | MemoryProt::GpuRead | + MemoryProt::GpuWrite | MemoryProt::GpuReadWrite; - if ((prot & ~valid_flags) != MemoryProt::NoAccess) { - LOG_ERROR(Core, "Invalid protection flags, prot: {:#x}, GpuWrite: {:#x}", - static_cast(prot), static_cast(MemoryProt::GpuWrite)); + MemoryProt invalid_flags = prot & ~valid_flags; + if (u32(invalid_flags) != 0 && u32(invalid_flags) != u32(MemoryProt::NoAccess)) { + LOG_ERROR(Core, "Invalid protection flags: prot = {:#x}, invalid flags = {:#x}", u32(prot), + invalid_flags); return ORBIS_KERNEL_ERROR_EINVAL; } @@ -356,19 +360,19 @@ int MemoryManager::MTypeProtect(VAddr addr, size_t size, VMAType mtype, MemoryPr // Set permissions Core::MemoryPermission perms{}; - if ((prot & MemoryProt::CpuRead) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::CpuRead)) { perms |= Core::MemoryPermission::Read; } - if ((prot & MemoryProt::CpuReadWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::CpuReadWrite)) { perms |= Core::MemoryPermission::ReadWrite; } - if ((prot & MemoryProt::GpuRead) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuRead)) { perms |= Core::MemoryPermission::Read; } - if ((prot & MemoryProt::GpuWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuWrite)) { perms |= Core::MemoryPermission::Write; } - if ((prot & MemoryProt::GpuReadWrite) != MemoryProt::NoAccess) { + if (True(prot & MemoryProt::GpuReadWrite)) { perms |= Core::MemoryPermission::ReadWrite; }