Security fixes for xrt:api leet audit tickets

src/runtime_src/core/common/api/aie/xrt_graph.cpp

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 // Copyright (C) 2020-2021 Xilinx, Inc. All rights reserved.
-// Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.
+// Copyright (C) 2023-2026 Advanced Micro Devices, Inc. All rights reserved.
 
 // This file implements XRT APIs as declared in
 // core/include/experimental/xrt_aie.h -- end user APIs
@@ -18,6 +18,7 @@
 #include "core/include/xcl_graph.h"
 
 #include "core/common/api/device_int.h"
+#include "core/common/api/handle.h"
 #include "core/common/api/native_profile.h"
 #include "core/common/device.h"
 #include "core/common/error.h"
@@ -257,7 +258,7 @@ namespace {
 
 // C-API Graph handles are inserted to this map.
 // Note: xrtGraphClose must be explicitly called before xclClose.
-static std::map<xrtGraphHandle, std::shared_ptr<xrt::graph_impl>> graph_cache;
+static xrt_core::handle_map<xrtGraphHandle, std::shared_ptr<xrt::graph_impl>> graph_cache;
 
 static std::shared_ptr<xrt::graph_impl>
 open_graph(xrtDeviceHandle dhdl, const xuid_t xclbin_uuid, const char* graph_name, xrt::graph::access_mode am)
@@ -272,17 +273,13 @@ open_graph(xrtDeviceHandle dhdl, const xuid_t xclbin_uuid, const char* graph_nam
 static std::shared_ptr<xrt::graph_impl>
 get_graph_hdl(xrtGraphHandle graph_handle)
 {
-  auto itr = graph_cache.find(graph_handle);
-  if (itr == graph_cache.end())
-    throw xrt_core::error(-EINVAL, "No such graph handle");
-  return (*itr).second;
+  return graph_cache.get_or_error(graph_handle);
 }
 
 static void
 close_graph(xrtGraphHandle hdl)
 {
-  if (graph_cache.erase(hdl) == 0)
-    throw std::runtime_error("Unexpected internal error");
+  graph_cache.remove_or_error(hdl);
 }
 
 static void
@@ -323,7 +320,7 @@ wait_gmio(xrtDeviceHandle dhdl, const char *gmio_name)
 }
 
 // C-API Profiling handles are inserted to this map.
-static std::map<int, std::shared_ptr<xrt::aie::profiling_impl>> profiling_cache;
+static xrt_core::handle_map<int, std::shared_ptr<xrt::aie::profiling_impl>> profiling_cache;
 
 static std::shared_ptr<xrt::aie::profiling_impl>
 create_profiling_event(xrtDeviceHandle dhdl)
@@ -592,8 +589,9 @@ xrtGraphOpen(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, const char* g
 {
   try {
     auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::primary);
-    graph_cache[hdl.get()] = hdl;
-    return hdl.get();
+    auto key = hdl.get();
+    graph_cache.add(key, std::move(hdl));
+    return key;
   }
   catch (const xrt_core::error& ex) {
     xrt_core::send_exception_message(ex.what());
@@ -610,8 +608,9 @@ xrtGraphOpenExclusive(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, cons
 {
   try {
     auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::exclusive);
-    graph_cache[hdl.get()] = hdl;
-    return hdl.get();
+    auto key = hdl.get();
+    graph_cache.add(key, std::move(hdl));
+    return key;
   }
   catch (const xrt_core::error& ex) {
     xrt_core::send_exception_message(ex.what());
@@ -628,8 +627,9 @@ xrtGraphOpenShared(xrtDeviceHandle dev_handle, const xuid_t xclbin_uuid, const c
 {
   try {
     auto hdl = open_graph(dev_handle, xclbin_uuid, graph_name, xrt::graph::access_mode::shared);
-    graph_cache[hdl.get()] = hdl;
-    return hdl.get();
+    auto key = hdl.get();
+    graph_cache.add(key, std::move(hdl));
+    return key;
   }
   catch (const xrt_core::error& ex) {
     xrt_core::send_exception_message(ex.what());
@@ -1024,7 +1024,7 @@ xrtAIEStartProfiling(xrtDeviceHandle handle, int option, const char *port1Name,
     const std::string port2 = port2Name ? port2Name : "";
     auto hdl = event->start(option, port1, port2, value);
     if (hdl != xrt::aie::profiling_impl::invalid_handle) {
-      profiling_cache[hdl] = std::move(event);
+      profiling_cache.add(hdl, std::move(event));
       return hdl;
     }
     else
@@ -1053,11 +1053,7 @@ uint64_t
 xrtAIEReadProfiling(xrtDeviceHandle /*handle*/, int pHandle)
 {
   try {
-    auto it = profiling_cache.find(pHandle);
-    if (it != profiling_cache.end())
-      return it->second->read();
-    else
-      throw xrt_core::error(-EINVAL, "No such profiling handle");
+    return profiling_cache.get_or_error(pHandle)->read();
   }
   catch (const xrt_core::error& ex) {
     xrt_core::send_exception_message(ex.what());
@@ -1083,13 +1079,8 @@ void
 xrtAIEStopProfiling(xrtDeviceHandle /*handle*/, int pHandle)
 {
   try {
-    auto it = profiling_cache.find(pHandle);
-    if (it != profiling_cache.end()) {
-      it->second->stop();
-      profiling_cache.erase(pHandle);
-    }
-    else
-      throw xrt_core::error(-EINVAL, "No such profiling handle");
+    profiling_cache.get_or_error(pHandle)->stop();
+    profiling_cache.remove_or_error(pHandle);
   }
   catch (const xrt_core::error& ex) {
     xrt_core::send_exception_message(ex.what());

src/runtime_src/core/common/api/handle.h

@@ -46,7 +46,7 @@ class handle_map<HandleType, std::shared_ptr<ImplType>>
     return handles;
   }
 
-  const std::shared_ptr<ImplType>&
+  std::shared_ptr<ImplType>
   get_or_error(HandleType handle) const
   {
     std::lock_guard<std::mutex> lk(mutex);

src/runtime_src/core/common/api/hw_queue.cpp

@@ -119,6 +119,7 @@ class command_manager
   std::mutex work_mutex;
   std::condition_variable work_cond;
   command_queue_type submitted_cmds;
+  std::vector<xrt_core::command*> running_cmds;  // guarded by work_mutex
   bool stop = false;
 
   // thread can be constructed only after data members are initialized
@@ -135,7 +136,6 @@ class command_manager
   monitor_loop()
   {
     std::vector<xrt_core::command*> busy_cmds;
-    std::vector<xrt_core::command*> running_cmds;
 
     while (true) {
 
@@ -271,11 +271,20 @@ class command_manager
       m_impl->submit(cmd);
     }
     catch (...) {
-      // Remove the pending command
+      // Remove the specific command to avoid two races:
+      // 1. monitor may have already drained submitted_cmds into running_cmds
+      // 2. a concurrent launch() may have pushed another command, making
+      //    pop_back() remove the wrong entry
       std::lock_guard<std::mutex> lk(work_mutex);
-      assert(get_command_state(cmd)==ERT_CMD_STATE_NEW);
-      if (!submitted_cmds.empty())
-        submitted_cmds.pop_back();
+      assert(get_command_state(cmd) == ERT_CMD_STATE_NEW);
+      auto it = std::find(submitted_cmds.begin(), submitted_cmds.end(), cmd);
+      if (it != submitted_cmds.end())
+        submitted_cmds.erase(it);
+      else {
+        auto rit = std::find(running_cmds.begin(), running_cmds.end(), cmd);
+        if (rit != running_cmds.end())
+          running_cmds.erase(rit);
+      }
       throw;
     }
 

src/runtime_src/core/common/api/xrt_bo.cpp

@@ -1073,7 +1073,7 @@ namespace {
 // deleted if no other shared ptrs exists for this buffer
 static xrt_core::handle_map<xrtBufferHandle, std::shared_ptr<xrt::bo_impl>> bo_cache;
 
-static const std::shared_ptr<xrt::bo_impl>&
+static std::shared_ptr<xrt::bo_impl>
 get_boh(xrtBufferHandle bhdl)
 {
   return bo_cache.get_or_error(bhdl);

src/runtime_src/core/common/api/xrt_error.cpp

@@ -336,6 +336,9 @@ xrtErrorGetString(xrtDeviceHandle, xrtErrorCode error, char* out, size_t len, si
       if (!out)
         return 0;
 
+      if (len == 0)
+        return -EINVAL;
+
       auto cp_len = std::min(len - 1, str.size());
       std::memcpy(out, str.c_str(), cp_len);
       out[cp_len] = 0;

src/runtime_src/core/common/api/xrt_kernel.cpp

@@ -5169,7 +5169,16 @@ aie_error_message_v1(const ert_packet* epkt, const std::string& msg,
         << "\nnumber of uC reported = " << std::dec
         << ctx_health->aie4.num_uc;
 
-    for (uint32_t i = 0; i < ctx_health->aie4.num_uc; ++i) {
+    // clamp num_uc to entries that fit in the packet payload; fixed overhead
+    // is version+npu_gen+ctx_state+num_uc+ctx_error_type = 5 uint32_t words
+    constexpr uint32_t aie4_fixed_words = 5;
+    constexpr uint32_t uc_entry_words = sizeof(ert_uc_health_info) / sizeof(uint32_t);
+    const uint32_t max_uc = (epkt->count > aie4_fixed_words)
+      ? (epkt->count - aie4_fixed_words) / uc_entry_words
+      : 0;
+    const uint32_t num_uc = std::min(max_uc, ctx_health->aie4.num_uc);
+
+    for (uint32_t i = 0; i < num_uc; ++i) {
       oss << "\nuc_info[" << i << "]: "
           << "\nuc_idx=0x" << std::setw(indent8) << std::hex
           << ctx_health->aie4.uc_info[i].uc_idx

src/runtime_src/core/common/api/xrt_xclbin.cpp

@@ -234,26 +234,35 @@ class xclbin::ip_impl
   void
   add_mem_at_idx(int32_t argidx, const xclbin::mem& mem)
   {
-    resize_args(argidx + 1);
+    // cast before +1 to avoid signed overflow when argidx == INT32_MAX
+    resize_args(static_cast<size_t>(argidx) + 1);
     if (!m_args[argidx])
       m_args[argidx] = xclbin::arg{std::make_shared<arg_impl>()};
     m_args[argidx].get_handle()->add_mem(mem);
   }
 
 public:
-  ip_impl(const ::connectivity* conn,
+  ip_impl(const ::connectivity* conn, size_t conn_size,
           const std::vector<xclbin::mem>& mems,
           const ::ip_data* ip, int32_t ipidx)
     : m_ip(ip), m_ip_layout_idx(ipidx)
   {
-    if (!conn)
+    // m_count (int32_t) is the only non-array member, so sizeof(int32_t)
+    // is the minimum section size needed to read from struct connectivity
+    if (!conn || conn_size < sizeof(int32_t) || conn->m_count < 0)
       return;
 
-    for (int32_t idx = 0; idx < conn->m_count; ++idx) {
+    auto max_count = (conn_size - sizeof(int32_t)) / sizeof(::connection);
+    auto count = std::min(static_cast<size_t>(conn->m_count), max_count);
+    for (int32_t idx = 0; idx < static_cast<int32_t>(count); ++idx) {
         auto& cxn = conn->m_connection[idx];
         if (cxn.m_ip_layout_index != m_ip_layout_idx)
           continue;
 
+        if (cxn.arg_index < 0 || cxn.mem_data_index < 0 ||
+            static_cast<size_t>(cxn.mem_data_index) >= mems.size())
+          throw std::runtime_error("xclbin: invalid connectivity index");
+
         add_mem_at_idx(cxn.arg_index, mems.at(cxn.mem_data_index));
     }
   }
@@ -412,12 +421,23 @@ class xclbin_impl
     init_mems(const xclbin_impl* ximpl)
     {
       std::vector<xclbin::mem> mems;
-      if (auto mem_topology = ximpl->get_section<const ::mem_topology*>(ASK_GROUP_TOPOLOGY)) {
-        mems.reserve(mem_topology->m_count);
-        for (int32_t idx = 0; idx < mem_topology->m_count; ++idx) {
-          auto mem = mem_topology->m_mem_data + idx;
-          mems.emplace_back(std::make_shared<xclbin::mem_impl>(mem, idx));
-        }
+      auto [data, size] = ximpl->get_axlf_section(ASK_GROUP_TOPOLOGY);
+      // m_count is the only non-array member, so sizeof(int32_t) is both the
+      // minimum section size needed to read from struct mem_topology and also
+      // the byte offset to m_mem_data.
+      if (!data || size < sizeof(int32_t))
+        return mems;
+
+      auto mem_topology = reinterpret_cast<const ::mem_topology*>(data);
+      if (mem_topology->m_count < 0)
+        return mems;
+
+      auto max_count = (size - sizeof(int32_t)) / sizeof(::mem_data);
+      auto count = std::min(static_cast<size_t>(mem_topology->m_count), max_count);
+      mems.reserve(count);
+      for (int32_t idx = 0; idx < static_cast<int32_t>(count); ++idx) {
+        auto mem = mem_topology->m_mem_data + idx;
+        mems.emplace_back(std::make_shared<xclbin::mem_impl>(mem, idx));
       }
       return mems;
     }
@@ -462,18 +482,29 @@ class xclbin_impl
     static std::vector<xclbin::ip>
     init_ips(const xclbin_impl* ximpl, const std::vector<xclbin::mem>& mems)
     {
-      auto ip_layout = ximpl->get_section<const ::ip_layout*>(IP_LAYOUT);
-      if (!ip_layout)
+      auto [ip_data, ip_size] = ximpl->get_axlf_section(IP_LAYOUT);
+      // m_count is the only non-array member, so sizeof(int32_t) is both the
+      // minimum section size needed to read from struct ip_layout and also
+      // the byte offset to m_ip_data.
+      if (!ip_data || ip_size < sizeof(int32_t))
         return {};
 
-      auto conn = ximpl->get_section<const ::connectivity*>(ASK_GROUP_CONNECTIVITY);
+      auto ip_layout = reinterpret_cast<const ::ip_layout*>(ip_data);
+      if (ip_layout->m_count < 0)
+        return {};
+
+      auto max_count = (ip_size - sizeof(int32_t)) / sizeof(::ip_data);
+      auto count = std::min(static_cast<size_t>(ip_layout->m_count), max_count);
+
+      auto [conn_data, conn_size] = ximpl->get_axlf_section(ASK_GROUP_CONNECTIVITY);
+      auto conn = reinterpret_cast<const ::connectivity*>(conn_data);
 
       std::vector<xclbin::ip> ips;
-      ips.reserve(ip_layout->m_count);
-      for (int32_t idx = 0; idx < ip_layout->m_count; ++idx)
+      ips.reserve(count);
+      for (int32_t idx = 0; idx < static_cast<int32_t>(count); ++idx)
         ips.emplace_back
           (std::make_shared<xclbin::ip_impl>
-           (conn, mems, ip_layout->m_ip_data + idx, idx));
+           (conn, conn_size, mems, ip_layout->m_ip_data + idx, idx));
 
       return ips;
     }

src/runtime_src/core/edge/user/shim.cpp

@@ -434,7 +434,7 @@ xclGetDeviceInfo2(xclDeviceInfo2 *info)
     mVBNV >> deviceName;
   }
   mVBNV.close();
-  std::size_t length = deviceName.copy(info->mName, deviceName.length(),0);
+  std::size_t length = deviceName.copy(info->mName, sizeof(info->mName) - 1, 0);
   info->mName[length] = '\0';
   return 0;
 }

src/runtime_src/hip/core/graph.h

@@ -91,7 +91,7 @@ class graph
     return m_node_list;
   }
 
-  const std::shared_ptr<graph_node>&
+  std::shared_ptr<graph_node>
   get_node(node_handle node_handle) const
   {
     return m_node_cache.get_or_error(node_handle);