oneapi-src / oneapi-src/unified-runtime

race condition in native cpu adaptor allocation functions

Open
#2,070 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

native-cpu
Dominant language
C++
Stars
57
Forks
120
Avg merge
1d 14h
Merged PRs (30d)
1

Description

There's a subtle race condition in get_alloc_info_entry.
get_alloc_info_entry protects the set of known allocations by grabbing the mutex protecting the same, but then returns a reference to the allocation by which point the mutex has been released.

It's plausible that by the time the caller dereferences the returned reference another thread may have freed the given allocation.

At first I thought that this might be enough:

diff --git a/source/adapters/native_cpu/context.hpp b/source/adapters/native_cpu/context.hpp
index c59ab4ea..d3f54b3d 100644
--- a/source/adapters/native_cpu/context.hpp
+++ b/source/adapters/native_cpu/context.hpp
@@ -110,7 +110,7 @@ struct ur_context_handle_t_ : RefCounted {
   }

   // Note this is made non-const to access the mutex
-  const native_cpu::usm_alloc_info &get_alloc_info_entry(const void *ptr) {
+  const native_cpu::usm_alloc_info get_alloc_info_entry(const void *ptr) {
     std::lock_guard<std::mutex> lock(alloc_mutex);
     auto it = allocations.find(ptr);
     if (it == allocations.end()) {

However, returning a value type here doesn't actually help since although it'll prevent a segfault or similar UB, there's no way to determine whether the information returned is still valid.

I'm not sure whether it's something we see in the wild, but it's a real possibility so might be worth a different approach

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in source/adapters/native_cpu/context.hpp at get_alloc_info_entry and trace its callers, along with the allocation mutex and allocation lifetime handling. Determine how concurrent lookup and free can safely coexist, then verify that callers cannot use invalid allocation metadata during concurrent operations.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.