From 3d317af34bea0869e50636f95bfd809eba57d41f Mon Sep 17 00:00:00 2001 From: "Gainullin, Artur" Date: Fri, 24 Apr 2026 15:18:27 -0700 Subject: [PATCH] [XPTI] Fix data races in tracepoint lookup and deletion Several functions in the Tracepoints class accessed MUID64Check and dereferenced tracepoint pointers without holding MTracepointMutex, allowing use-after-free when concurrent threads perform lookups (xptiLookupEvent) and deletions (xptiDeleteTracepoint). Fixes: - lookupEventData: acquire shared lock before MUID64Check access - releaseEvent: acquire exclusive lock before MUID64Check access (was only locked for the erase/delete, not the preceding check) - isValidUID64: acquire shared lock before MUID64Check access - deleteTracepoint: move TP->MUId read inside the exclusive lock - findEvent, queryPayloadByUID, lookupPayload: move into implemenation into new thread-safe Tracepoints methods --- xptifw/src/xpti_trace_framework.cpp | 110 +++++++++++++++++----------- 1 file changed, 66 insertions(+), 44 deletions(-) diff --git a/xptifw/src/xpti_trace_framework.cpp b/xptifw/src/xpti_trace_framework.cpp index 35c3e0c15d84b..6db822ad36754 100644 --- a/xptifw/src/xpti_trace_framework.cpp +++ b/xptifw/src/xpti_trace_framework.cpp @@ -862,6 +862,8 @@ class Tracepoints { if (!UId) return nullptr; + xpti::SharedLock Lock(MTracepointMutex); + if (MUID64Check.count(UId) == 0) return nullptr; @@ -914,6 +916,8 @@ class Tracepoints { if (!Event || Event->unique_id == xpti::invalid_uid) return; + std::lock_guard Lock(MTracepointMutex); + if (MUID64Check.count(Event->unique_id) == 0) return; @@ -922,18 +926,15 @@ class Tracepoints { reinterpret_cast(Event->unique_id); xpti::uid128_t UId = TP->MUId; - { - std::lock_guard Lock(MTracepointMutex); - // Find the event list for a given UID - auto &Instances = MTracepoints[UId]; - MUID64Check.erase(UId.uid64); - // Now release the event associated with the UID instance - delete Instances[UId.instance]; - Instances.erase(UId.instance); - // If there are no more events associated with the UID, we can release - // the Payload as well, but we will not as the same payload may be - // revisited and we need to keep the instance count going - } + // Find the event list for a given UID + auto &Instances = MTracepoints[UId]; + MUID64Check.erase(UId.uid64); + // Now release the event associated with the UID instance + delete Instances[UId.instance]; + Instances.erase(UId.instance); + // If there are no more events associated with the UID, we can release + // the Payload as well, but we will not as the same payload may be + // revisited and we need to keep the instance count going } /// @brief Adds metadata to a trace event. @@ -1166,9 +1167,9 @@ class Tracepoints { return xpti::result_t::XPTI_RESULT_INVALIDARG; { - xpti::uid128_t UId = TP->MUId; // Lock the mutex to ensure thread-safe access to the tracepoints map std::lock_guard Lock(MTracepointMutex); + xpti::uid128_t UId = TP->MUId; // Find the tracepoint for a given UID auto &Instances = MTracepoints[UId]; // Now release the 64-bit UID associated with tracepoint instance @@ -1316,7 +1317,55 @@ class Tracepoints { /// @return Returns true if the UID exists in the set, indicating it is valid; /// otherwise, returns false. /// - bool isValidUID64(uint64_t UId) { return (MUID64Check.count(UId) > 0); } + bool isValidUID64(uint64_t UId) { + xpti::SharedLock Lock(MTracepointMutex); + return (MUID64Check.count(UId) > 0); + } + + const xpti::trace_event_data_t *findEvent(uint64_t UId) { + if (UId == xpti::invalid_uid) + return nullptr; + + xpti::SharedLock Lock(MTracepointMutex); + + if (MUID64Check.count(UId) == 0) + return nullptr; + + xpti::TracePointImpl *TP = reinterpret_cast(UId); + if (TP && xpti::is_valid_event(&TP->MEvent)) + return &TP->MEvent; + return nullptr; + } + + const xpti::payload_t *queryPayloadByUID(uint64_t UId) { + if (UId == xpti::invalid_uid) + return nullptr; + + xpti::SharedLock Lock(MTracepointMutex); + + if (MUID64Check.count(UId) == 0) + return nullptr; + + xpti::TracePointImpl *TP = reinterpret_cast(UId); + if (TP && xpti::is_valid_payload(&TP->MPayload)) + return &TP->MPayload; + return nullptr; + } + + const xpti_payload_t *lookupPayload(uint64_t UId) { + if (UId == xpti::invalid_uid) + return nullptr; + + xpti::SharedLock Lock(MTracepointMutex); + + if (MUID64Check.count(UId) == 0) + return nullptr; + + xpti::TracePointImpl *TP = reinterpret_cast(UId); + if (TP && xpti::is_valid_payload(&TP->MPayload)) + return TP->payload(); + return nullptr; + } private: /// @brief Registers an event with a given payload and returns the event @@ -2217,19 +2266,7 @@ class Framework { /// nullptr otherwise. /// inline const xpti::trace_event_data_t *findEvent(uint64_t UniversalID) { - if (UniversalID == xpti::invalid_uid) - return nullptr; - - if (MTracepoints.isValidUID64(UniversalID)) { - xpti::TracePointImpl *TP = - reinterpret_cast(UniversalID); - if (TP && xpti::is_valid_event(&TP->MEvent)) - return &TP->MEvent; - else - return nullptr; - } - - return nullptr; + return MTracepoints.findEvent(UniversalID); } /// @@ -2486,26 +2523,11 @@ class Framework { } const xpti::payload_t *queryPayloadByUID(uint64_t uid) { - if (uid == xpti::invalid_uid) - return nullptr; - if (!MTracepoints.isValidUID64(uid)) - return nullptr; - - xpti::TracePointImpl *TP = reinterpret_cast(uid); - if (xpti::is_valid_payload(&TP->MPayload)) - return &TP->MPayload; - - return nullptr; + return MTracepoints.queryPayloadByUID(uid); } inline const xpti_payload_t *lookupPayload(uint64_t uid) { - if (!MTracepoints.isValidUID64(uid)) - return nullptr; - xpti::TracePointImpl *TP = reinterpret_cast(uid); - if (TP && xpti::is_valid_payload(&TP->MPayload)) - return TP->payload(); - else - return nullptr; + return MTracepoints.lookupPayload(uid); } void printStatistics() {