From bfe958055136e65757508845b68599ffa1aa2b78 Mon Sep 17 00:00:00 2001 From: Nekotekina Date: Fri, 6 Nov 2020 11:55:01 +0300 Subject: [PATCH] atomic.cpp: fix cond_handle data structures Fix a critical bug with possible id out of range. --- rpcs3/util/atomic.cpp | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/rpcs3/util/atomic.cpp b/rpcs3/util/atomic.cpp index 1068bdea68..7aae5177f6 100644 --- a/rpcs3/util/atomic.cpp +++ b/rpcs3/util/atomic.cpp @@ -193,13 +193,13 @@ namespace atomic_wait } // Max allowed thread number is chosen to fit in 16 bits -static std::aligned_storage_t s_cond_list[UINT16_MAX]{}; +static std::aligned_storage_t s_cond_list[UINT16_MAX + 1]{}; // Used to allow concurrent notifying -static atomic_t s_cond_refs[UINT16_MAX + 1]{}; +static atomic_t s_cond_refs[UINT16_MAX + 1]{}; // Allocation bits -static atomic_t s_cond_bits[::align(UINT16_MAX, 64) / 64]{}; +static atomic_t s_cond_bits[(UINT16_MAX + 1) / 64]{}; // Allocation semaphore static atomic_t s_cond_sema{0}; @@ -237,18 +237,24 @@ static u32 cond_alloc() return false; }); - if (ok) + if (ok) [[likely]] { // Find lowest clear bit const u32 id = group * 64 + std::countr_one(bits); + if (id == 0) [[unlikely]] + { + // Special case, set bit and continue + continue; + } + // Construct inplace before it can be used new (s_cond_list + id) atomic_wait::cond_handle(); // Add first reference verify(HERE), !s_cond_refs[id]++; - return id + 1; + return id; } } @@ -261,7 +267,7 @@ static atomic_wait::cond_handle* cond_get(u32 cond_id) { if (cond_id - 1 < u32{UINT16_MAX}) [[likely]] { - return std::launder(reinterpret_cast(s_cond_list + (cond_id - 1))); + return std::launder(reinterpret_cast(s_cond_list + cond_id)); } return nullptr; @@ -276,7 +282,7 @@ static void cond_free(u32 cond_id) } // Dereference, destroy on last ref - if (--s_cond_refs[cond_id - 1]) + if (--s_cond_refs[cond_id]) { return; } @@ -285,7 +291,7 @@ static void cond_free(u32 cond_id) cond_get(cond_id)->~cond_handle(); // Remove the allocation bit - s_cond_bits[(cond_id - 1) / 64] &= ~(1ull << ((cond_id - 1) % 64)); + s_cond_bits[cond_id / 64] &= ~(1ull << (cond_id % 64)); // Release the semaphore s_cond_sema--; @@ -295,9 +301,9 @@ static u32 cond_lock(atomic_t* sema) { while (const u32 cond_id = sema->load()) { - const auto [old, ok] = s_cond_refs[cond_id - 1].fetch_op([](u16& ref) + const auto [old, ok] = s_cond_refs[cond_id].fetch_op([](u32& ref) { - if (!ref || ref == UINT16_MAX) + if (!ref || ref == UINT32_MAX) { // Don't reference already deallocated semaphore return false; @@ -312,9 +318,9 @@ static u32 cond_lock(atomic_t* sema) return cond_id; } - if (old == UINT16_MAX) + if (old == UINT32_MAX) { - fmt::raw_error("Thread limit " STRINGIZE(UINT16_MAX) " for a single address reached in atomic notifier."); + fmt::raw_error("Thread limit " STRINGIZE(UINT32_MAX) " for a single address reached in atomic notifier."); } if (sema->load() != cond_id)