fix(amdsmi): keep the memory partition name inside the caller's buffer

projects/amdsmi/CHANGELOG.md

@@ -21,6 +21,9 @@ Full documentation for amd_smi_lib is available at [https://rocm.docs.amd.com/pr
 
 ### Resolved Issues
 
+- **Fixed a one-byte overrun when reading the memory partition into a small buffer**.  
+  - A buffer too small for the partition name had the byte after its end overwritten. The name is now truncated inside the buffer, and the call still reports that the buffer is too small.
+
 - **Fixed crashes when several threads start or stop AMD SMI at the same time**.  
   - Programs that initialize and shut down the library from more than one thread could crash, find no GPUs, or leave the library initialized after every thread had shut it down.
 

projects/amdsmi/rocm_smi/src/rocm_smi.cc

@@ -6784,7 +6784,7 @@ rsmi_status_t rsmi_dev_memory_partition_get(uint32_t dv_ind, char* memory_partit
     return ret;
   }
 
-  std::size_t buff_size = returning_memory_partition.copy(memory_partition, len);
+  std::size_t buff_size = returning_memory_partition.copy(memory_partition, len - 1);
   memory_partition[buff_size] = '\0';
 
   if (len < (returning_memory_partition.size() + 1)) {

projects/amdsmi/tests/amd_smi_test/functional/gpu/partition/memorypartition_read.h

@@ -0,0 +1,32 @@
+// Copyright Advanced Micro Devices, Inc.
+// SPDX-License-Identifier: MIT
+
+#ifndef TESTS_AMD_SMI_TEST_FUNCTIONAL_MEMORYPARTITION_READ_H_
+#define TESTS_AMD_SMI_TEST_FUNCTIONAL_MEMORYPARTITION_READ_H_
+
+#include "test_base.h"
+
+class TestMemoryPartitionRead : public TestBase {
+ public:
+  TestMemoryPartitionRead();
+
+  // @Brief: Destructor for test case of TestMemoryPartitionRead
+  virtual ~TestMemoryPartitionRead();
+
+  // @Brief: Setup the environment for measurement
+  virtual void SetUp();
+
+  // @Brief: Core measurement execution
+  virtual void Run();
+
+  // @Brief: Clean up and retrieve the resource
+  virtual void Close();
+
+  // @Brief: Display results
+  virtual void DisplayResults() const;
+
+  // @Brief: Display information about what this test does
+  virtual void DisplayTestInfo(void);
+};
+
+#endif  // TESTS_AMD_SMI_TEST_FUNCTIONAL_MEMORYPARTITION_READ_H_

projects/amdsmi/tests/amd_smi_test/functional/gpu/partition/memorypartition_read_test.cc

@@ -0,0 +1,86 @@
+// Copyright Advanced Micro Devices, Inc.
+// SPDX-License-Identifier: MIT
+
+#include "memorypartition_read.h"
+
+#include <gtest/gtest.h>
+
+#include <cstdint>
+#include <cstring>
+#include <initializer_list>
+#include <iostream>
+#include <string>
+#include <vector>
+
+#include "amd_smi/amdsmi.h"
+#include "test_base.h"
+#include "test_common.h"
+
+TestMemoryPartitionRead::TestMemoryPartitionRead() : TestBase() {
+  set_title("AMDSMI Memory Partition Read Test");
+  set_description(
+      "Reads the current memory partition into buffers of several sizes and "
+      "verifies that the result always stays inside the caller's buffer.");
+}
+
+TestMemoryPartitionRead::~TestMemoryPartitionRead(void) {}
+
+void TestMemoryPartitionRead::SetUp(void) { TestBase::SetUp(); }
+
+void TestMemoryPartitionRead::DisplayTestInfo(void) { TestBase::DisplayTestInfo(); }
+
+void TestMemoryPartitionRead::DisplayResults(void) const { TestBase::DisplayResults(); }
+
+void TestMemoryPartitionRead::Close() { TestBase::Close(); }
+
+void TestMemoryPartitionRead::Run(void) {
+  constexpr uint32_t k255Len = 255;
+  constexpr char kGuard = '#';
+
+  TestBase::Run();
+  PRINT_VERBOSITY();
+  if (setup_failed_) {
+    std::cout << "** SetUp Failed for this test. Skipping.**" << std::endl;
+    return;
+  }
+
+  for (uint32_t dv_ind = 0; dv_ind < num_monitor_devs(); ++dv_ind) {
+    PrintDeviceHeader(processor_handles_[dv_ind]);
+
+    char full[k255Len] = {};
+    amdsmi_status_t err =
+        amdsmi_get_gpu_memory_partition(processor_handles_[dv_ind], full, k255Len);
+    if (err == AMDSMI_STATUS_NOT_SUPPORTED) {
+      IF_VERB(STANDARD) {
+        std::cout << "\t**Memory partition is not supported on this device" << std::endl;
+      }
+      continue;
+    }
+    ASSERT_EQ(err, AMDSMI_STATUS_SUCCESS);
+    const std::string name(full);
+    if (name.empty()) {
+      IF_VERB(STANDARD) {
+        std::cout << "\t**Memory partition is empty on this device" << std::endl;
+      }
+      continue;
+    }
+    IF_VERB(STANDARD) { std::cout << "\t**Current memory partition: " << name << std::endl; }
+
+    // Each buffer has `len` usable bytes followed by one guard byte. A buffer
+    // too small for the name must hold a terminated prefix of it, and a buffer
+    // that fits must hold all of it. The guard byte must stay untouched.
+    const auto name_len = static_cast<uint32_t>(name.size());
+    for (uint32_t len : {1u, 2u, name_len, name_len + 1}) {
+      std::vector<char> buf(len + 1, kGuard);
+      err = amdsmi_get_gpu_memory_partition(processor_handles_[dv_ind], buf.data(), len);
+      const bool fits = len > name_len;
+      EXPECT_EQ(err, fits ? AMDSMI_STATUS_SUCCESS : AMDSMI_STATUS_INSUFFICIENT_SIZE)
+          << "len=" << len;
+      EXPECT_EQ(buf[len], kGuard) << "wrote past the end of a " << len << "-byte buffer";
+      const size_t used = strnlen(buf.data(), len);
+      EXPECT_LT(used, len) << "no terminator inside a " << len << "-byte buffer";
+      EXPECT_EQ(std::string(buf.data(), used), fits ? name : name.substr(0, len - 1))
+          << "len=" << len;
+    }
+  }
+}

projects/amdsmi/tests/amd_smi_test/main.cc

@@ -26,6 +26,7 @@
 #include "functional/gpu/metrics/process_list_read.h"
 #include "functional/gpu/partition/computepartition_memallocmode_read_write.h"
 #include "functional/gpu/partition/computepartition_read_write.h"
+#include "functional/gpu/partition/memorypartition_read.h"
 #include "functional/gpu/partition/memorypartition_read_write.h"
 #include "functional/gpu/pci/pci_read_write.h"
 #include "functional/gpu/perf/overdrive_read.h"
@@ -248,6 +249,10 @@ TEST(GpuFunctionalReadOnly, TestGpuPartitionMetricsRead) {
   TestGpuPartitionMetricsRead tst;
   RunGenericTest(&tst);
 }
+TEST(GpuFunctionalReadOnly, TestMemoryPartitionRead) {
+  TestMemoryPartitionRead tst;
+  RunGenericTest(&tst);
+}
 TEST(GpuFunctionalReadOnly, TestMetricsCounterRead) {
   TestMetricsCounterRead tst;
   RunGenericTest(&tst);

projects/rocm-smi-lib/src/rocm_smi.cc

@@ -5119,7 +5119,7 @@ rsmi_status_t rsmi_dev_memory_partition_get(uint32_t dv_ind, char* memory_partit
     return ret;
   }
 
-  std::size_t buff_size = returning_memory_partition.copy(memory_partition, len);
+  std::size_t buff_size = returning_memory_partition.copy(memory_partition, len - 1);
   memory_partition[buff_size] = '\0';
 
   if (len < (returning_memory_partition.size() + 1)) {

projects/rocm-smi-lib/tests/rocm_smi_test/functional/memorypartition_read_write.cc

@@ -191,18 +191,22 @@ void TestMemoryPartitionReadWrite::Run(void) {
     }
     ASSERT_TRUE(ret == RSMI_STATUS_SUCCESS);
 
-    // Verify api support checking functionality is working
-    uint32_t kLen = 2;
-    char smallBuffer[kLen];
+    // Verify api support checking functionality is working. The byte after
+    // the kLen usable bytes is a guard: the name must be truncated and
+    // terminated inside kLen, with nothing written past it.
+    constexpr uint32_t kLen = 2;
+    constexpr char kGuard = '#';
+    char smallBuffer[kLen + 1];
+    smallBuffer[kLen] = kGuard;
     err = rsmi_dev_memory_partition_get(dv_ind, smallBuffer, kLen);
-    size_t size = sizeof(smallBuffer) / sizeof(*smallBuffer);
     ASSERT_EQ(err, RSMI_STATUS_INSUFFICIENT_SIZE);
-    ASSERT_EQ((size_t)kLen, size);
+    ASSERT_EQ(smallBuffer[kLen], kGuard);
+    ASSERT_EQ(smallBuffer[kLen - 1], '\0');
     if (err == RSMI_STATUS_INSUFFICIENT_SIZE) {
       IF_VERB(STANDARD) {
         std::cout << "\t**"
                   << "Confirmed RSMI_STATUS_INSUFFICIENT_SIZE was returned "
-                  << "and size matches kLen requested." << std::endl;
+                  << "and nothing was written past kLen." << std::endl;
       }
     }