diff --git a/projects/rocm-smi-lib/CMakeLists.txt b/projects/rocm-smi-lib/CMakeLists.txt index aef929496b..be2db4cd36 100755 --- a/projects/rocm-smi-lib/CMakeLists.txt +++ b/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") diff --git a/projects/rocm-smi-lib/example/rocm_smi_example.cc b/projects/rocm-smi-lib/example/rocm_smi_example.cc index d78e6fcd87..be1415a101 100755 --- a/projects/rocm-smi-lib/example/rocm_smi_example.cc +++ b/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(1.1 * static_cast(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(0.95 * static_cast(new_speed)) && + cur_spd < static_cast(1.1 * static_cast(new_speed))) || + (cur_spd > + static_cast(0.95 * static_cast(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(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); diff --git a/projects/rocm-smi-lib/include/rocm_smi/rocm_smi.h b/projects/rocm-smi-lib/include/rocm_smi/rocm_smi.h index 93b78d67de..8f6cae797d 100755 --- a/projects/rocm-smi-lib/include/rocm_smi/rocm_smi.h +++ b/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 diff --git a/projects/rocm-smi-lib/include/rocm_smi/rocm_smi_exception.h b/projects/rocm-smi-lib/include/rocm_smi/rocm_smi_exception.h index 470a37845d..7c898fb958 100755 --- a/projects/rocm-smi-lib/include/rocm_smi/rocm_smi_exception.h +++ b/projects/rocm-smi-lib/include/rocm_smi/rocm_smi_exception.h @@ -49,6 +49,14 @@ #include #include +#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 { diff --git a/projects/rocm-smi-lib/src/rocm_smi.cc b/projects/rocm-smi-lib/src/rocm_smi.cc index 91f1747383..99e218efad 100755 --- a/projects/rocm-smi-lib/src/rocm_smi.cc +++ b/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 &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 &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 &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 &freq_lines, lanes[i] = std::stoi(star_str.substr(1), nullptr); } } - return freq*multiplier; + return static_cast(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(freq*multiplier); multiplier = get_multiplier_from_str(volts_units_str[0]); - pt->voltage = volts*multiplier; + pt->voltage = static_cast(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(lo*multiplier); multiplier = get_multiplier_from_str(hi_units_str[0]); - rg->upper_bound = hi*multiplier; + rg->upper_bound = static_cast(hi*multiplier); return; } - /** * Parse a string of the form " <|*>" */ @@ -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 *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(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(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(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(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(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((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 *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(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 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(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(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(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(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(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(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(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(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(val_vec.size()); } if (records == nullptr) { return RSMI_STATUS_SUCCESS; diff --git a/projects/rocm-smi-lib/src/rocm_smi_counters.cc b/projects/rocm-smi-lib/src/rocm_smi_counters.cc index 6bde1c8e61..b575741f7b 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_counters.cc +++ b/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(start_bit); + val->field_size = static_cast(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(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(-ret); } if (ret != sizeof(perf_read_format_t)) { diff --git a/projects/rocm-smi-lib/src/rocm_smi_device.cc b/projects/rocm-smi-lib/src/rocm_smi_device.cc index 81318667a0..a4ba2db753 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_device.cc +++ b/projects/rocm-smi-lib/src/rocm_smi_device.cc @@ -461,6 +461,10 @@ static const std::map 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 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 *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; } diff --git a/projects/rocm-smi-lib/src/rocm_smi_main.cc b/projects/rocm-smi-lib/src/rocm_smi_main.cc index 08d35dda5a..8b3bcd5dd8 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_main.cc +++ b/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 diff --git a/projects/rocm-smi-lib/src/rocm_smi_monitor.cc b/projects/rocm-smi-lib/src/rocm_smi_monitor.cc index b23f0682b9..a31ca937b3 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_monitor.cc +++ b/projects/rocm-smi-lib/src/rocm_smi_monitor.cc @@ -224,6 +224,9 @@ static const std::map kMonFuncDependsMap = { Monitor::Monitor(std::string path, RocmSMI_env_vars const *e) : path_(path), env_(e) { +#ifdef NDEBUG + env_ = nullptr; +#endif } Monitor::~Monitor(void) { } diff --git a/projects/rocm-smi-lib/src/rocm_smi_power_mon.cc b/projects/rocm-smi-lib/src/rocm_smi_power_mon.cc index cc8c177ce1..f843caa782 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_power_mon.cc +++ b/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(0xFFFFFFFFFFFFFFFF)/1000) { + throw amd::smi::rsmi_exception(RSMI_STATUS_UNEXPECTED_DATA, + __FUNCTION__); + } + *val = static_cast(num_units * 1000); // Convert W to mW break; default: diff --git a/projects/rocm-smi-lib/src/rocm_smi_utils.cc b/projects/rocm-smi-lib/src/rocm_smi_utils.cc index c12e0b7de0..a4630e092d 100755 --- a/projects/rocm-smi-lib/src/rocm_smi_utils.cc +++ b/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