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 runtime fatal CPERs reporting no AFIDs**.  
   - `amd-smi ras --cper` showed an empty `list afids` column for fatal records, `amd-smi ras --afid --cper-file` printed `-`, and `amdsmi_get_afids_from_cper()` returned no AFIDs. amdgpu writes fatal crashdump sections 32 bytes shorter than `sizeof(cper_sec_crashdump)`, and the section bounds check required the full struct, so every such section was skipped. The check now requires only the dump member the record type uses.
 

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)) {