Security improvements

Improvements include
* adding additional build flags that warn about stack-smashing
and type conversion errors
* run-time checks for valid function input values and adquate
space for the result of arithmetic operations.
* make sure default case for switch statements do something
besides just assert
* disable using env. var. debugging in release mode

Change-Id: I5f048310c5c56e05d9ec31bcc273404d6a0dd646


[ROCm/rocm_smi_lib commit: d00b9ac07d]

projects/rocm-smi-lib/CMakeLists.txt

@@ -86,8 +86,25 @@ set(CPACK_PACKAGE_FILE_NAME "${ROCM_SMI_PACKAGE}-${PKG_VERSION_STR}")
 set(CMAKE_VERBOSE_MAKEFILE on)
 
 ## Compiler flags
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -fno-rtti -m64")
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -msse -msse2 -std=c++11 ")
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -Wall -Wextra -fno-rtti -m64 -msse -msse2 -std=c++11 ")
+# Security options
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -Wconversion -Wcast-align ")
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -Wformat=2 -fno-common -Wstrict-overflow -Wtrampolines ")
+ # Intentionally leave out -Wsign-promo. It causes spurious warnings.
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS}  -Woverloaded-virtual -Wreorder ")
+
+## Security breach mitigation flags
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -DFORTIFY_SOURCE=2 -fstack-protector-all -Wcast-align")
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -Wl,-z,noexecstack -Wl,-znoexecheap -Wl,-z,relro ")
+set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -Wl,-z,now -fPIE")
+
 # Use this instead of above for 32 bit
 # set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -m32")
 

projects/rocm-smi-lib/example/rocm_smi_example.cc

@@ -56,16 +56,16 @@
 #define CHK_RSMI_RET(RET) { \
   if (RET != RSMI_STATUS_SUCCESS) { \
     const char *err_str; \
-    std::cout << "RSMI call returned " << RET \
+    std::cout << "RSMI call returned " << (RET) \
       << " at line " << __LINE__ << std::endl; \
-      rsmi_status_string(RET, &err_str); \
+      rsmi_status_string((RET), &err_str); \
       std::cout << err_str << std::endl; \
     return RET; \
   } \
 }
 
 #define CHK_RSMI_PERM_RET(RET) { \
-    if (RET == RSMI_STATUS_PERMISSION) { \
+    if ((RET) == RSMI_STATUS_PERMISSION) { \
       std::cout << "This command requires root access." << std::endl; \
     } else { \
       CHK_RSMI_RET(RET) \
@@ -257,7 +257,7 @@ static rsmi_status_t test_set_fan_speed(uint32_t dv_ind) {
   rsmi_status_t ret;
   int64_t orig_speed;
   int64_t new_speed;
-  int64_t cur_speed;
+  int64_t cur_spd;
 
   print_test_header("Fan Speed Control", dv_ind);
 
@@ -270,7 +270,7 @@ static rsmi_status_t test_set_fan_speed(uint32_t dv_ind) {
     return RSMI_STATUS_SUCCESS;
   }
 
-  new_speed = 1.1 * orig_speed;
+  new_speed = static_cast<uint64_t>(1.1 * static_cast<double>(orig_speed));
 
   std::cout << "Setting fan speed to " << new_speed << std::endl;
 
@@ -279,13 +279,16 @@ static rsmi_status_t test_set_fan_speed(uint32_t dv_ind) {
 
   sleep(4);
 
-  ret = rsmi_dev_fan_speed_get(dv_ind, 0, &cur_speed);
+  ret = rsmi_dev_fan_speed_get(dv_ind, 0, &cur_spd);
   CHK_RSMI_RET(ret)
 
-  std::cout << "New fan speed: " << cur_speed << std::endl;
+  std::cout << "New fan speed: " << cur_spd << std::endl;
 
-  assert((cur_speed > 0.95 * new_speed && cur_speed < 1.1 * new_speed) ||
-      (cur_speed > 0.95 * RSMI_MAX_FAN_SPEED));
+  assert(
+      (cur_spd > static_cast<int64_t>(0.95 * static_cast<double>(new_speed)) &&
+       cur_spd < static_cast<int64_t>(1.1 * static_cast<double>(new_speed))) ||
+       (cur_spd >
+       static_cast<int64_t>(0.95 * static_cast<double>(RSMI_MAX_FAN_SPEED))));
 
   std::cout << "Resetting fan control to auto..." << std::endl;
 
@@ -294,10 +297,10 @@ static rsmi_status_t test_set_fan_speed(uint32_t dv_ind) {
 
   sleep(3);
 
-  ret = rsmi_dev_fan_speed_get(dv_ind, 0, &cur_speed);
+  ret = rsmi_dev_fan_speed_get(dv_ind, 0, &cur_spd);
   CHK_RSMI_RET(ret)
 
-  std::cout << "End fan speed: " << cur_speed << std::endl;
+  std::cout << "End fan speed: " << cur_spd << std::endl;
 
   return ret;
 }
@@ -448,7 +451,7 @@ int main() {
     ret = rsmi_dev_fan_speed_max_get(i, 0, &val_ui64);
     CHK_RSMI_RET(ret)
     std::cout << "\t**Current Fan Speed: ";
-    std::cout << val_i64/static_cast<float>(val_ui64)*100;
+    std::cout << val_i64/val_ui64*100;
     std::cout << "% ("<< val_i64 << "/" << val_ui64 << ")" << std::endl;
 
     ret = rsmi_dev_fan_rpms_get(i, 0, &val_i64);

projects/rocm-smi-lib/include/rocm_smi/rocm_smi.h

@@ -116,7 +116,8 @@ typedef enum {
                                          //!< was read
   RSMI_STATUS_NO_DATA,                   //!< No data was found for a given
                                          //!< input
-
+  RSMI_STATUS_UNEXPECTED_DATA,           //!< The data read or provided to
+                                         //!< function is not what was expected
   RSMI_STATUS_UNKNOWN_ERROR = 0xFFFFFFFF,  //!< An unknown error occurred
 } rsmi_status_t;
 
@@ -867,7 +868,7 @@ rsmi_status_t rsmi_dev_name_get(uint32_t dv_ind, char *name, size_t len);
  *  provided arguments.
  *
  *  @param[in] len the length of the caller provided buffer @p brand.
-
+ *
  *  @retval ::RSMI_STATUS_SUCCESS call was successful
  *  @retval ::RSMI_STATUS_NOT_SUPPORTED installed software or hardware does not
  *  support this function with the given arguments
@@ -901,7 +902,7 @@ rsmi_status_t rsmi_dev_brand_get(uint32_t dv_ind, char *brand, uint32_t len);
  *  ::RSMI_STATUS_INVALID_ARGS if the function is supported with the provided,
  *  arguments and ::RSMI_STATUS_NOT_SUPPORTED if it is not supported with the
  *  provided arguments.
-
+ *
  *  @param[in] len the length of the caller provided buffer @p name.
  *
  *  @retval ::RSMI_STATUS_SUCCESS call was successful

projects/rocm-smi-lib/include/rocm_smi/rocm_smi_exception.h

@@ -49,6 +49,14 @@
 #include <exception>
 #include <string>
 
+#include "rocm_smi/rocm_smi.h"
+
+#define THROW_IF_NULLPTR_DEREF(PTR) \
+  assert((PTR) != nullptr); \
+  if ((PTR) == nullptr) { \
+    throw amd::smi::rsmi_exception(RSMI_STATUS_INVALID_ARGS, __FUNCTION__); \
+  }
+
 namespace amd {
 namespace smi {
 

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

@@ -79,7 +79,6 @@ static rsmi_status_t handleException() {
   } catch (const amd::smi::rsmi_exception& e) {
     debug_print("Exception caught: %s.\n", e.what());
     return e.error_code();
-    return RSMI_STATUS_INTERNAL_EXCEPTION;
   } catch (const std::exception& e) {
     debug_print("Unhandled exception: %s\n", e.what());
     assert(false && "Unhandled exception.");
@@ -135,7 +134,7 @@ static rsmi_status_t handleException() {
 
 #define CHK_SUPPORT(RT_PTR, VR, SUB_VR)  \
     GET_DEV_FROM_INDX \
-    CHK_API_SUPPORT_ONLY(RT_PTR, VR, SUB_VR)
+    CHK_API_SUPPORT_ONLY((RT_PTR), (VR), (SUB_VR))
 
 #define CHK_SUPPORT_NAME_ONLY(RT_PTR) \
     CHK_SUPPORT((RT_PTR), RSMI_DEFAULT_VARIANT, RSMI_DEFAULT_VARIANT) \
@@ -198,6 +197,7 @@ static uint64_t get_multiplier_from_str(char units_char) {
 
     default:
       assert(!"Unexpected units for frequency");
+      throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_DATA, __FUNCTION__);
   }
   return multiplier;
 }
@@ -211,7 +211,7 @@ static uint64_t freq_string_to_int(const std::vector<std::string> &freq_lines,
   std::istringstream fs(freq_lines[i]);
 
   uint32_t ind;
-  long double freq;
+  float freq;
   std::string junk;
   std::string units_str;
   std::string star_str;
@@ -222,6 +222,10 @@ static uint64_t freq_string_to_int(const std::vector<std::string> &freq_lines,
   fs >> units_str;
   fs >> star_str;
 
+  if (freq < 0) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_SIZE, __FUNCTION__);
+  }
+
   if (is_curr != nullptr) {
     if (freq_lines[i].find("*") != std::string::npos) {
       *is_curr = true;
@@ -229,7 +233,7 @@ static uint64_t freq_string_to_int(const std::vector<std::string> &freq_lines,
       *is_curr = false;
     }
   }
-  uint32_t multiplier = get_multiplier_from_str(units_str[0]);
+  long double multiplier = get_multiplier_from_str(units_str[0]);
 
   if (star_str[0] == 'x') {
     assert(lanes != nullptr && "Lanes are provided but null lanes pointer");
@@ -237,7 +241,7 @@ static uint64_t freq_string_to_int(const std::vector<std::string> &freq_lines,
       lanes[i] = std::stoi(star_str.substr(1), nullptr);
     }
   }
-  return freq*multiplier;
+  return static_cast<uint64_t>(freq*multiplier);
 }
 
 static void freq_volt_string_to_point(std::string in_line,
@@ -245,10 +249,11 @@ static void freq_volt_string_to_point(std::string in_line,
   std::istringstream fs_vlt(in_line);
 
   assert(pt != nullptr);
+  THROW_IF_NULLPTR_DEREF(pt)
 
   uint32_t ind;
-  long double freq;
-  long double volts;
+  float freq;
+  float volts;
   std::string junk;
   std::string freq_units_str;
   std::string volts_units_str;
@@ -260,12 +265,16 @@ static void freq_volt_string_to_point(std::string in_line,
   fs_vlt >> volts;
   fs_vlt >> volts_units_str;
 
-  uint32_t multiplier = get_multiplier_from_str(freq_units_str[0]);
+  if (freq < 0) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_SIZE, __FUNCTION__);
+  }
 
-  pt->frequency = freq*multiplier;
+  long double multiplier = get_multiplier_from_str(freq_units_str[0]);
+
+  pt->frequency = static_cast<uint64_t>(freq*multiplier);
 
   multiplier = get_multiplier_from_str(volts_units_str[0]);
-  pt->voltage = volts*multiplier;
+  pt->voltage = static_cast<uint64_t>(volts*multiplier);
 
   return;
 }
@@ -274,10 +283,11 @@ static void od_value_pair_str_to_range(std::string in_line, rsmi_range_t *rg) {
   std::istringstream fs_rng(in_line);
 
   assert(rg != nullptr);
+  THROW_IF_NULLPTR_DEREF(rg)
 
   std::string clk;
-  uint64_t lo;
-  uint64_t hi;
+  float lo;
+  float hi;
   std::string lo_units_str;
   std::string hi_units_str;
 
@@ -287,17 +297,16 @@ static void od_value_pair_str_to_range(std::string in_line, rsmi_range_t *rg) {
   fs_rng >> hi;
   fs_rng >> hi_units_str;
 
-  uint32_t multiplier = get_multiplier_from_str(lo_units_str[0]);
+  long double multiplier = get_multiplier_from_str(lo_units_str[0]);
 
-  rg->lower_bound = lo*multiplier;
+  rg->lower_bound = static_cast<uint64_t>(lo*multiplier);
 
   multiplier = get_multiplier_from_str(hi_units_str[0]);
-  rg->upper_bound = hi*multiplier;
+  rg->upper_bound = static_cast<uint64_t>(hi*multiplier);
 
   return;
 }
 
-
 /**
  * Parse a string of the form "<int index> <mode name string> <|*>"
  */
@@ -308,6 +317,8 @@ power_prof_string_to_int(std::string pow_prof_line, bool *is_curr,
   std::string mode;
   size_t tmp;
 
+  THROW_IF_NULLPTR_DEREF(prof_ind)
+
   rsmi_power_profile_preset_masks_t ret = RSMI_PWR_PROF_PRST_INVALID;
 
   fs >> *prof_ind;
@@ -348,6 +359,10 @@ power_prof_string_to_int(std::string pow_prof_line, bool *is_curr,
 
 static rsmi_status_t get_dev_value_str(amd::smi::DevInfoTypes type,
                                       uint32_t dv_ind, std::string *val_str) {
+  assert(val_str != nullptr);
+  if (val_str == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
   GET_DEV_FROM_INDX
   int ret = dev->readDevInfo(type, val_str);
 
@@ -355,6 +370,10 @@ static rsmi_status_t get_dev_value_str(amd::smi::DevInfoTypes type,
 }
 static rsmi_status_t get_dev_value_int(amd::smi::DevInfoTypes type,
                                          uint32_t dv_ind, uint64_t *val_int) {
+  assert(val_int != nullptr);
+  if (val_int == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
   GET_DEV_FROM_INDX
   int ret = dev->readDevInfo(type, val_int);
 
@@ -363,6 +382,10 @@ static rsmi_status_t get_dev_value_int(amd::smi::DevInfoTypes type,
 
 static rsmi_status_t get_dev_value_line(amd::smi::DevInfoTypes type,
                                       uint32_t dv_ind, std::string *val_str) {
+  assert(val_str != nullptr);
+  if (val_str == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
   GET_DEV_FROM_INDX
   int ret = dev->readDevInfoLine(type, val_str);
 
@@ -379,6 +402,10 @@ static rsmi_status_t set_dev_value(amd::smi::DevInfoTypes type,
 
 static rsmi_status_t get_dev_mon_value(amd::smi::MonitorTypes type,
                          uint32_t dv_ind, uint32_t sensor_ind, int64_t *val) {
+  assert(val != nullptr);
+  if (val == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
   GET_DEV_FROM_INDX
 
   assert(dev->monitor() != nullptr);
@@ -397,8 +424,12 @@ static rsmi_status_t get_dev_mon_value(amd::smi::MonitorTypes type,
 
 static rsmi_status_t get_dev_mon_value(amd::smi::MonitorTypes type,
                         uint32_t dv_ind, uint32_t sensor_ind, uint64_t *val) {
-  GET_DEV_FROM_INDX
+  assert(val != nullptr);
+  if (val == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
 
+  GET_DEV_FROM_INDX
   assert(dev->monitor() != nullptr);
 
   std::string val_str;
@@ -456,6 +487,10 @@ static rsmi_status_t get_power_mon_value(amd::smi::PowerMonTypes type,
 
 static rsmi_status_t get_dev_value_vec(amd::smi::DevInfoTypes type,
                          uint32_t dv_ind, std::vector<std::string> *val_vec) {
+  assert(val_vec != nullptr);
+  if (val_vec == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
   GET_DEV_FROM_INDX
 
   int ret = dev->readDevInfo(type, val_vec);
@@ -493,13 +528,14 @@ rsmi_shut_down(void) {
 rsmi_status_t
 rsmi_num_monitor_devices(uint32_t *num_devices) {
   TRY
+  assert(num_devices != nullptr);
   if (num_devices == nullptr) {
     return RSMI_STATUS_INVALID_ARGS;
   }
 
   amd::smi::RocmSMI& smi = amd::smi::RocmSMI::getInstance();
 
-  *num_devices = smi.monitor_devices().size();
+  *num_devices = static_cast<uint32_t>(smi.monitor_devices().size());
   return RSMI_STATUS_SUCCESS;
   CATCH
 }
@@ -682,6 +718,12 @@ static rsmi_status_t
 get_id(uint32_t dv_ind, amd::smi::DevInfoTypes typ, uint16_t *id) {
   TRY
   std::string val_str;
+  int64_t val_u64;
+
+  assert(id != nullptr);
+  if (id == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
 
   DEVICE_MUTEX
 
@@ -692,10 +734,17 @@ get_id(uint32_t dv_ind, amd::smi::DevInfoTypes typ, uint16_t *id) {
   }
 
   errno = 0;
-  *id = strtoul(val_str.c_str(), nullptr, 16);
+  val_u64 = strtoul(val_str.c_str(), nullptr, 16);
   assert(errno == 0);
+  if (errno != 0) {
+    return errno_to_rsmi_status(errno);
+  }
+  if (val_u64 > 0xFFFF) {
+    return RSMI_STATUS_UNEXPECTED_SIZE;
+  }
+  *id = static_cast<uint16_t>(val_u64);
 
-  return errno_to_rsmi_status(errno);
+  return RSMI_STATUS_SUCCESS;
   CATCH
 }
 
@@ -761,7 +810,13 @@ rsmi_dev_overdrive_level_get(uint32_t dv_ind, uint32_t *od) {
   }
 
   errno = 0;
-  *od = strtoul(val_str.c_str(), nullptr, 10);
+  int64_t val_ul = strtoul(val_str.c_str(), nullptr, 10);
+
+  if (val_ul > 0xFFFFFFFF) {
+    return RSMI_STATUS_UNEXPECTED_SIZE;
+  }
+
+  *od = static_cast<uint32_t>(val_ul);
   assert(errno == 0);
 
   return RSMI_STATUS_SUCCESS;
@@ -815,7 +870,7 @@ static rsmi_status_t get_frequencies(amd::smi::DevInfoTypes type,
     return RSMI_STATUS_NOT_YET_IMPLEMENTED;
   }
 
-  f->num_supported = val_vec.size();
+  f->num_supported = static_cast<uint32_t>(val_vec.size());
   bool current = false;
   f->current = RSMI_MAX_NUM_FREQUENCIES + 1;  // init to an invalid value
 
@@ -860,8 +915,11 @@ static rsmi_status_t get_power_profiles(uint32_t dv_ind,
     return ret;
   }
   assert(val_vec.size() <= RSMI_MAX_NUM_POWER_PROFILES);
-
-  p->num_profiles = val_vec.size() - 1;  // -1 for the header line
+  if (val_vec.size() > RSMI_MAX_NUM_POWER_PROFILES + 1 || val_vec.size() < 1) {
+    return RSMI_STATUS_UNEXPECTED_SIZE;
+  }
+  // -1 for the header line, below
+  p->num_profiles = static_cast<uint32_t>(val_vec.size() - 1);
   bool current = false;
   p->current = RSMI_PWR_PROF_PRST_INVALID;  // init to an invalid value
   p->available_profiles = 0;
@@ -932,6 +990,9 @@ static rsmi_status_t get_od_clk_volt_info(uint32_t dv_ind,
   rsmi_status_t ret;
 
   assert(p != nullptr);
+  if (p == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
 
   ret = get_dev_value_vec(amd::smi::kDevPowerODVoltage, dv_ind, &val_vec);
   if (ret != RSMI_STATUS_SUCCESS) {
@@ -945,6 +1006,9 @@ static rsmi_status_t get_od_clk_volt_info(uint32_t dv_ind,
   }
 
   assert(val_vec[kOD_SCLK_label_array_index] == "OD_SCLK:");
+  if (val_vec[kOD_SCLK_label_array_index] != "OD_SCLK:") {
+    return RSMI_STATUS_UNEXPECTED_DATA;
+  }
 
   p->curr_sclk_range.lower_bound = freq_string_to_int(val_vec, nullptr,
                                      nullptr, kOD_SCLK_label_array_index + 1);
@@ -962,6 +1026,9 @@ static rsmi_status_t get_od_clk_volt_info(uint32_t dv_ind,
                                      nullptr, kOD_MCLK_label_array_index + 1);
 
   assert(val_vec[kOD_VDDC_CURVE_label_array_index] == "OD_VDDC_CURVE:");
+  if (val_vec[kOD_VDDC_CURVE_label_array_index] != "OD_VDDC_CURVE:") {
+    return RSMI_STATUS_UNEXPECTED_DATA;
+  }
 
   uint32_t tmp = kOD_VDDC_CURVE_label_array_index + 1;
   for (uint32_t i = 0; i < RSMI_NUM_VOLTAGE_CURVE_POINTS; ++i) {
@@ -969,13 +1036,22 @@ static rsmi_status_t get_od_clk_volt_info(uint32_t dv_ind,
   }
 
   assert(val_vec[kOD_OD_RANGE_label_array_index] == "OD_RANGE:");
+  if (val_vec[kOD_OD_RANGE_label_array_index] != "OD_RANGE:") {
+    return RSMI_STATUS_UNEXPECTED_DATA;
+  }
+
   od_value_pair_str_to_range(val_vec[kOD_OD_RANGE_label_array_index + 1],
                                                       &(p->sclk_freq_limits));
   od_value_pair_str_to_range(val_vec[kOD_OD_RANGE_label_array_index + 2],
                                                       &(p->mclk_freq_limits));
 
   assert((val_vec.size() - kOD_VDDC_CURVE_start_index)%2 == 0);
-  p->num_regions = (val_vec.size() - kOD_VDDC_CURVE_start_index) / 2;
+  if ((val_vec.size() - kOD_VDDC_CURVE_start_index)%2 != 0) {
+    return RSMI_STATUS_UNEXPECTED_SIZE;
+  }
+
+  p->num_regions =
+     static_cast<uint32_t>((val_vec.size() - kOD_VDDC_CURVE_start_index) / 2);
 
   return RSMI_STATUS_SUCCESS;
   CATCH
@@ -985,10 +1061,16 @@ static void get_vc_region(uint32_t start_ind,
                 std::vector<std::string> *val_vec, rsmi_freq_volt_region_t *p) {
   assert(p != nullptr);
   assert(val_vec != nullptr);
+  THROW_IF_NULLPTR_DEREF(p)
+  THROW_IF_NULLPTR_DEREF(val_vec)
+
   // There must be at least 1 region to read in
   assert(val_vec->size() >= kOD_OD_RANGE_label_array_index + 2);
   assert((*val_vec)[kOD_OD_RANGE_label_array_index] == "OD_RANGE:");
-
+  if ((val_vec->size() < kOD_OD_RANGE_label_array_index + 2)  ||
+      ((*val_vec)[kOD_OD_RANGE_label_array_index] != "OD_RANGE:") ) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_DATA, __FUNCTION__);
+  }
   od_value_pair_str_to_range((*val_vec)[start_ind], &p->freq_range);
   od_value_pair_str_to_range((*val_vec)[start_ind + 1], &p->volt_range);
   return;
@@ -997,10 +1079,10 @@ static void get_vc_region(uint32_t start_ind,
 /*
  * num_regions [inout] on calling, the number of regions requested to be read
  * in. At completion, the number of regions actually read in
- * 
+ *
  * p [inout] point to pre-allocated memory where function will write region
  * values. Caller must make sure there is enough space for at least
- * *num_regions regions. On 
+ * *num_regions regions.
  */
 static rsmi_status_t get_od_clk_volt_curve_regions(uint32_t dv_ind,
                             uint32_t *num_regions, rsmi_freq_volt_region_t *p) {
@@ -1010,6 +1092,8 @@ static rsmi_status_t get_od_clk_volt_curve_regions(uint32_t dv_ind,
 
   assert(num_regions != nullptr);
   assert(p != nullptr);
+  THROW_IF_NULLPTR_DEREF(p)
+  THROW_IF_NULLPTR_DEREF(num_regions)
 
   ret = get_dev_value_vec(amd::smi::kDevPowerODVoltage, dv_ind, &val_vec);
   if (ret != RSMI_STATUS_SUCCESS) {
@@ -1022,9 +1106,15 @@ static rsmi_status_t get_od_clk_volt_curve_regions(uint32_t dv_ind,
     return RSMI_STATUS_NOT_YET_IMPLEMENTED;
   }
 
-  uint32_t val_vec_size = val_vec.size();
+  uint32_t val_vec_size = static_cast<uint32_t>(val_vec.size());
   assert((val_vec_size - kOD_VDDC_CURVE_start_index) > 0);
   assert((val_vec_size - kOD_VDDC_CURVE_start_index)%2 == 0);
+
+  if (((val_vec_size - kOD_VDDC_CURVE_start_index) <= 0)  ||
+      (((val_vec_size - kOD_VDDC_CURVE_start_index)%2 != 0))) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_SIZE, __FUNCTION__);
+  }
+
   *num_regions = std::min((val_vec_size - kOD_VDDC_CURVE_start_index) / 2,
                                                                 *num_regions);
 
@@ -1184,7 +1274,10 @@ rsmi_dev_firmware_version_get(uint32_t dv_ind, rsmi_fw_block_t block,
     case RSMI_FW_BLOCK_VCN:
       dev_type = amd::smi::kDevFwVersionVcn;
       break;
+    default:
+      return RSMI_STATUS_INVALID_ARGS;
   }
+
   ret = get_dev_value_int(dev_type, dv_ind, fw_version);
   if (ret != 0) {
     return errno_to_rsmi_status(ret);
@@ -1199,6 +1292,10 @@ static std::string bitfield_to_freq_string(uint64_t bitf,
   std::string bf_str("");
   std::bitset<RSMI_MAX_NUM_FREQUENCIES> bs(bitf);
 
+  if (num_supported > RSMI_MAX_NUM_FREQUENCIES) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_INVALID_ARGS, __FUNCTION__);
+  }
+
   for (uint32_t i = 0; i < num_supported; ++i) {
     if (bs[i]) {
       bf_str += std::to_string(i);
@@ -1218,6 +1315,10 @@ rsmi_dev_gpu_clk_freq_set(uint32_t dv_ind,
   REQUIRE_ROOT_ACCESS
   DEVICE_MUTEX
 
+  if (clk_type > RSMI_CLK_TYPE_LAST) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
+
   ret = rsmi_dev_gpu_clk_freq_get(dv_ind, clk_type, &freqs);
 
   if (ret != RSMI_STATUS_SUCCESS) {
@@ -1225,6 +1326,9 @@ rsmi_dev_gpu_clk_freq_set(uint32_t dv_ind,
   }
 
   assert(freqs.num_supported <= RSMI_MAX_NUM_FREQUENCIES);
+  if (freqs.num_supported > RSMI_MAX_NUM_FREQUENCIES) {
+    return RSMI_STATUS_UNEXPECTED_SIZE;
+  }
 
   amd::smi::RocmSMI& smi = amd::smi::RocmSMI::getInstance();
 
@@ -1293,6 +1397,7 @@ get_id_name_str_from_line(uint64_t id, std::string ln,
   std::string ret_str;
 
   assert(ln_str != nullptr);
+  THROW_IF_NULLPTR_DEREF(ln_str)
 
   *ln_str >> token1;
   if (std::stoul(token1, nullptr, 16) == id) {
@@ -1307,6 +1412,11 @@ get_id_name_str_from_line(uint64_t id, std::string ln,
 static rsmi_status_t get_backup_name(uint16_t id, char *name, size_t len) {
   std::string name_str;
 
+  assert(name != nullptr);
+  if (name == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
+
   name_str += "0x";
 
   std::stringstream strm;
@@ -1344,6 +1454,10 @@ static rsmi_status_t get_dev_name_from_id(uint32_t dv_ind, char *name,
   assert(name != nullptr);
   assert(len > 0);
 
+  if (name == nullptr || len == 0) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
+
   name[0] = '\0';
 
   ret = rsmi_dev_vendor_id_get(dv_ind, &vendor_id);
@@ -1455,6 +1569,11 @@ static rsmi_status_t
 get_dev_drm_render_minor(uint32_t dv_ind, uint32_t *minor) {
   GET_DEV_FROM_INDX
 
+  assert(minor != nullptr);
+  if (minor == nullptr) {
+    return RSMI_STATUS_INVALID_ARGS;
+  }
+
   *minor = dev->drm_render_minor();
   if (*minor)
     return RSMI_STATUS_SUCCESS;
@@ -1508,7 +1627,7 @@ rsmi_dev_brand_get(uint32_t dv_ind, char *brand, uint32_t len) {
     // Find the brand name using sku_value
     it = brand_names.find(sku_value);
     if (it != brand_names.end()) {
-      uint32_t ln = it->second.copy(brand, len);
+      uint32_t ln = static_cast<uint32_t>(it->second.copy(brand, len));
       brand[std::min(len - 1, ln)] = '\0';
 
       if (len < (it->second.size() + 1)) {
@@ -1539,7 +1658,7 @@ rsmi_dev_vram_vendor_get(uint32_t dv_ind, char *brand, uint32_t len) {
     return errno_to_rsmi_status(ret);
   }
 
-  uint32_t ln = val_str.copy(brand, len);
+  uint32_t ln = static_cast<uint32_t>(val_str.copy(brand, len));
 
   brand[std::min(len - 1, ln)] = '\0';
 
@@ -1589,6 +1708,7 @@ rsmi_dev_vendor_name_get(uint32_t dv_ind, char *name, size_t len) {
   TRY
   CHK_SUPPORT_NAME_ONLY(name)
 
+  assert(len > 0);
   if (len == 0) {
     return RSMI_STATUS_INVALID_ARGS;
   }
@@ -1757,6 +1877,8 @@ rsmi_dev_temp_metric_get(uint32_t dv_ind, uint32_t sensor_type,
      return errno_to_rsmi_status(err);
   }
 
+  // getSensorIndex will throw an out of range exception if sensor_type is not
+  // found
   uint32_t sensor_index =
          m->getSensorIndex(static_cast<rsmi_temperature_type_t>(sensor_type));
 
@@ -2100,9 +2222,6 @@ rsmi_dev_memory_busy_percent_get(uint32_t dv_ind, uint32_t *busy_percent) {
   TRY
   rsmi_status_t ret;
 
-  if (busy_percent == nullptr) {
-    return RSMI_STATUS_INVALID_ARGS;
-  }
   CHK_SUPPORT_NAME_ONLY(busy_percent)
 
   uint64_t tmp_util = 0;
@@ -2110,6 +2229,9 @@ rsmi_dev_memory_busy_percent_get(uint32_t dv_ind, uint32_t *busy_percent) {
   DEVICE_MUTEX
   ret = get_dev_value_int(amd::smi::kDevMemBusyPercent, dv_ind, &tmp_util);
 
+  if (tmp_util > 100) {
+    return RSMI_STATUS_UNEXPECTED_DATA;
+  }
   *busy_percent = static_cast<uint32_t>(tmp_util);
   return ret;
   CATCH
@@ -2216,7 +2338,11 @@ rsmi_dev_busy_percent_get(uint32_t dv_ind, uint32_t *busy_percent) {
   }
 
   errno = 0;
-  *busy_percent = strtoul(val_str.c_str(), nullptr, 10);
+  *busy_percent = static_cast<uint32_t>(strtoul(val_str.c_str(), nullptr, 10));
+
+  if (*busy_percent > 100) {
+    return RSMI_STATUS_UNEXPECTED_DATA;
+  }
   assert(errno == 0);
 
   return RSMI_STATUS_SUCCESS;
@@ -2242,7 +2368,7 @@ rsmi_dev_vbios_version_get(uint32_t dv_ind, char *vbios, uint32_t len) {
     return errno_to_rsmi_status(ret);
   }
 
-  uint32_t ln = val_str.copy(vbios, len);
+  uint32_t ln = static_cast<uint32_t>(val_str.copy(vbios, len));
 
   vbios[std::min(len - 1, ln)] = '\0';
 
@@ -2309,7 +2435,7 @@ rsmi_version_str_get(rsmi_sw_component_t component, char *ver_str,
     val_str = buf.release;
   }
 
-  uint32_t ln = val_str.copy(ver_str, len);
+  uint32_t ln = static_cast<uint32_t>(val_str.copy(ver_str, len));
 
   ver_str[std::min(len - 1, ln)] = '\0';
 
@@ -2338,7 +2464,7 @@ rsmi_status_t rsmi_dev_serial_number_get(uint32_t dv_ind,
     return ret;
   }
 
-  uint32_t ln = val_str.copy(serial_num, len);
+  uint32_t ln = static_cast<uint32_t>(val_str.copy(serial_num, len));
 
   serial_num[std::min(len - 1, ln)] = '\0';
 
@@ -2420,7 +2546,7 @@ rsmi_dev_counter_destroy(rsmi_event_handle_t evnt_handle) {
 
 rsmi_status_t
 rsmi_counter_control(rsmi_event_handle_t evt_handle,
-                                 rsmi_counter_command_t cmd, void *cmd_args) {
+                                 rsmi_counter_command_t cmd, void *) {
   TRY
 
   amd::smi::evt::Event *evt =
@@ -2430,11 +2556,7 @@ rsmi_counter_control(rsmi_event_handle_t evt_handle,
 
   REQUIRE_ROOT_ACCESS
 
-  uint32_t ret;
-
-  // This is for future command args. This would work in conjunction with a
-  // new function to set perf attributes.
-  (void) cmd_args;
+  uint32_t ret = 0;
 
   if (evt_handle == 0) {
     return RSMI_STATUS_INVALID_ARGS;
@@ -2451,6 +2573,7 @@ rsmi_counter_control(rsmi_event_handle_t evt_handle,
 
     default:
       assert(!"Unexpected perf counter command");
+      return RSMI_STATUS_INVALID_ARGS;
   }
   return errno_to_rsmi_status(ret);
 
@@ -2570,7 +2693,7 @@ rsmi_dev_memory_reserved_pages_get(uint32_t dv_ind, uint32_t *num_pages,
   }
 
   if (records == nullptr || *num_pages > val_vec.size()) {
-    *num_pages = val_vec.size();
+    *num_pages = static_cast<uint32_t>(val_vec.size());
   }
   if (records == nullptr) {
     return RSMI_STATUS_SUCCESS;

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

@@ -196,8 +196,15 @@ parse_field_config(std::string fstr, evnt_info_t *val) {
   assert(jnk == '-');
   ss >> end_bit;
 
-  val->start_bit = start_bit;
-  val->field_size = end_bit - start_bit + 1;
+  if (start_bit > end_bit ||
+      start_bit > 0xFF ||
+      end_bit > 0xFF ||
+      ((end_bit - start_bit + 1) > 0xFF)) {
+    throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_SIZE, __FUNCTION__);
+  }
+
+  val->start_bit = static_cast<uint8_t>(start_bit);
+  val->field_size = static_cast<uint8_t>(end_bit - start_bit + 1);
 }
 
 static uint32_t
@@ -313,12 +320,14 @@ amd::smi::evt::Event::openPerfHandle(void) {
   attr_.disabled = 1;
   attr_.inherit = 1;
 
-  fd_ = syscall(__NR_perf_event_open, &attr_,
+  int64_t p_ret = syscall(__NR_perf_event_open, &attr_,
                            -1, 0, -1, PERF_FLAG_FD_NO_GROUP);
 
-  if (fd_ < 0) {
+  if (p_ret < 0) {
     return errno;
   }
+
+  fd_ = static_cast<int>(p_ret);
   return 0;
 }
 
@@ -359,9 +368,9 @@ amd::smi::evt::Event::stopCounter(void) {
   return 0;
 }
 
-static ssize_t
+static long
 readn(int fd, void *buf, size_t n) {
-  size_t left = n;
+  ssize_t left = n;
   ssize_t bytes;
 
   while (left) {
@@ -388,7 +397,7 @@ amd::smi::evt::Event::getValue(rsmi_counter_value_t *val) {
   perf_read_format_t pvalue;
   ret = readn(fd_, &pvalue, sizeof(perf_read_format_t));
   if (ret < 0) {
-    return -ret;
+    return static_cast<uint32_t>(-ret);
   }
 
   if (ret != sizeof(perf_read_format_t)) {

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

@@ -461,6 +461,10 @@ static const std::map<const char *, dev_depends_t> kDevFuncDependsMap = {
 Device::Device(std::string p, RocmSMI_env_vars const *e) : path_(p), env_(e) {
   monitor_ = nullptr;
 
+#ifdef NDEBUG
+    env_ = nullptr;
+#endif
+
   // Get the device name
   size_t i = path_.rfind('/', path_.length());
   std::string dev = path_.substr(i + 1, path_.length() - i);
@@ -486,6 +490,7 @@ template <typename T>
 int Device::openSysfsFileStream(DevInfoTypes type, T *fs, const char *str) {
   auto sysfs_path = path_;
 
+#ifdef DEBUG
   if (env_->path_DRM_root_override && type == env_->enum_override) {
     sysfs_path = env_->path_DRM_root_override;
 
@@ -493,6 +498,7 @@ int Device::openSysfsFileStream(DevInfoTypes type, T *fs, const char *str) {
       sysfs_path += ".write";
     }
   }
+#endif
 
   sysfs_path += "/device/";
   sysfs_path += kDevAttribNameMap.at(type);
@@ -545,11 +551,8 @@ int Device::writeDevInfoStr(DevInfoTypes type, std::string valStr) {
     return ret;
   }
 
-  try {
-    fs << valStr;
-  } catch (...) {
-    std::cout << "Write to file threw exception" << std::endl;
-  }
+  // We'll catch any exceptions in rocm_smi.cc code.
+  fs << valStr;
   fs.close();
 
   return 0;
@@ -581,7 +584,7 @@ int Device::writeDevInfo(DevInfoTypes type, uint64_t val) {
       break;
 
     default:
-      break;
+      return EINVAL;
   }
 
   return -1;
@@ -599,7 +602,7 @@ int Device::writeDevInfo(DevInfoTypes type, std::string val) {
       return writeDevInfoStr(type, val);
 
     default:
-      break;
+      return EINVAL;
   }
 
   return -1;
@@ -709,7 +712,7 @@ int Device::readDevInfo(DevInfoTypes type, uint64_t *val) {
       break;
 
     default:
-      return -1;
+      return EINVAL;
   }
   return 0;
 }
@@ -734,7 +737,7 @@ int Device::readDevInfo(DevInfoTypes type, std::vector<std::string> *val) {
       break;
 
     default:
-      return -1;
+      return EINVAL;
   }
 
   return 0;
@@ -759,7 +762,7 @@ int Device::readDevInfo(DevInfoTypes type, std::string *val) {
       break;
 
     default:
-      return -1;
+      return EINVAL;
   }
   return 0;
 }

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

@@ -89,7 +89,7 @@ static uint32_t  GetDrmRenderMinor(const std::string s) {
   std::string drm_path = s;
   int drm_minor = 0;
   const std::string render_file_prefix = "renderD";
-  const uint32_t prefix_size = render_file_prefix.size();
+  const uint64_t prefix_size = render_file_prefix.size();
   drm_path += "/device/drm";
 
   auto drm_dir = opendir(drm_path.c_str());
@@ -289,21 +289,34 @@ RocmSMI& RocmSMI::getInstance(uint64_t flags) {
 }
 
 static int GetEnvVarInteger(const char *ev_str) {
+#ifdef NDEBUG
+  (void)ev_str;
+#else
   ev_str = getenv(ev_str);
 
   if (ev_str) {
     return atoi(ev_str);
   }
+#endif
   return 0;
 }
 
 // Get and store env. variables in this method
 void RocmSMI::GetEnvVariables(void) {
+#ifdef NDEBUG
+  (void)GetEnvVarInteger(nullptr);  // This is to quiet release build warning.
+  env_vars_.debug_output_bitfield = 0;
+  env_vars_.path_DRM_root_override = nullptr;
+  env_vars_.path_HWMon_root_override = nullptr;
+  env_vars_.path_power_root_override = nullptr;
+  env_vars_.enum_override = 0;
+#else
   env_vars_.debug_output_bitfield = GetEnvVarInteger("RSMI_DEBUG_BITFIELD");
   env_vars_.path_DRM_root_override   = getenv("RSMI_DEBUG_DRM_ROOT_OVERRIDE");
   env_vars_.path_HWMon_root_override = getenv("RSMI_DEBUG_HWMON_ROOT_OVERRIDE");
   env_vars_.path_power_root_override = getenv("RSMI_DEBUG_PP_ROOT_OVERRIDE");
   env_vars_.enum_override = GetEnvVarInteger("RSMI_DEBUG_ENUM_OVERRIDE");
+#endif
 }
 
 void

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

@@ -224,6 +224,9 @@ static const std::map<const char *, monitor_depends_t> kMonFuncDependsMap = {
 
   Monitor::Monitor(std::string path, RocmSMI_env_vars const *e) :
                                                         path_(path), env_(e) {
+#ifdef NDEBUG
+    env_ = nullptr;
+#endif
 }
 Monitor::~Monitor(void) {
 }

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

@@ -54,6 +54,7 @@
 #include "rocm_smi/rocm_smi_monitor.h"
 #include "rocm_smi/rocm_smi_utils.h"
 #include "rocm_smi/rocm_smi_common.h"
+#include "rocm_smi/rocm_smi_exception.h"
 
 namespace amd {
 namespace smi {
@@ -114,7 +115,16 @@ static int parse_power_str(std::string s, PowerMonTypes type, uint64_t *val) {
       l_ss >> num_units;
       l_ss >> sz;
       assert(sz == "W");  // We only expect Watts at this time
-      *val = num_units * 1000;  // Convert Watts to milliwatts
+      if (sz != "W") {
+        throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_DATA,
+                                                                __FUNCTION__);
+      }
+
+      if (num_units > static_cast<long double>(0xFFFFFFFFFFFFFFFF)/1000) {
+        throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_DATA,
+                                                                __FUNCTION__);
+      }
+      *val = static_cast<uint64_t>(num_units * 1000);  // Convert W to mW
       break;
 
     default:

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

@@ -140,17 +140,16 @@ int ReadSysfsStr(std::string path, std::string *retStr) {
   return ret;
 }
 
-bool IsInteger(const std::string & n_str)
-{
-   if(n_str.empty() || ((!isdigit(n_str[0])) && (n_str[0] != '-')
+bool IsInteger(const std::string & n_str) {
+  if (n_str.empty() || ((!isdigit(n_str[0])) && (n_str[0] != '-')
                                                       && (n_str[0] != '+'))) {
-     return false;
-   }
+    return false;
+  }
 
-   char * tmp;
-   strtol(n_str.c_str(), &tmp, 10);
+  char * tmp;
+  strtol(n_str.c_str(), &tmp, 10);
 
-   return (*tmp == 0);
+  return (*tmp == 0);
 }
 }  // namespace smi
 }  // namespace amd