From 12e2f5da63fcfdb544f87ec492e5b1bc4f89868c Mon Sep 17 00:00:00 2001 From: Alexander Amelkin Date: Fri, 19 Apr 2019 15:07:25 +0300 Subject: [PATCH] sdr: Fix segfault on invalid unit types The program would crash if the BMC returned an out of range (>90) unit type for a full sensor record. This commit adds a range check and also add support for IPMI 2.0 additional unit types 91 and 92 ("fatal error" and "grams"). Resolves ipmitool/ipmitool#118 Signed-off-by: Alexander Amelkin --- include/ipmitool/ipmi_sdr.h | 32 ++++++++++++++--- lib/ipmi_sdr.c | 72 +++++++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/include/ipmitool/ipmi_sdr.h b/include/ipmitool/ipmi_sdr.h index 5e6afd3..9f783c4 100644 --- a/include/ipmitool/ipmi_sdr.h +++ b/include/ipmitool/ipmi_sdr.h @@ -37,6 +37,7 @@ # include #endif +#include #include #include #include @@ -381,6 +382,29 @@ struct sdr_record_common_sensor { struct sdr_record_mask mask; +/* IPMI 2.0, Table 43-1, byte 21[7:6] Analog (numeric) Data Format */ +#define SDR_UNIT_FMT_UNSIGNED 0 /* unsigned */ +#define SDR_UNIT_FMT_1S_COMPL 1 /* 1's complement (signed) */ +#define SDR_UNIT_FMT_2S_COMPL 2 /* 2's complement (signed) */ +#define SDR_UNIT_FMT_NA 3 /* does not return analog (numeric) reading */ +/* IPMI 2.0, Table 43-1, byte 21[5:3] Rate */ +#define SDR_UNIT_RATE_NONE 0 /* none */ +#define SDR_UNIT_RATE_MICROSEC 1 /* per us */ +#define SDR_UNIT_RATE_MILLISEC 2 /* per ms */ +#define SDR_UNIT_RATE_SEC 3 /* per s */ +#define SDR_UNIT_RATE_MIN 4 /* per min */ +#define SDR_UNIT_RATE_HR 5 /* per hour */ +#define SDR_UNIT_RATE_DAY 6 /* per day */ +#define SDR_UNIT_RATE_RSVD 7 /* reserved */ +/* IPMI 2.0, Table 43-1, byte 21[2:1] Modifier Unit */ +#define SDR_UNIT_MOD_NONE 0 /* none */ +#define SDR_UNIT_MOD_DIV 1 /* Basic Unit / Modifier Unit */ +#define SDR_UNIT_MOD_MUL 2 /* Basic Unit * Mofifier Unit */ +#define SDR_UNIT_MOD_RSVD 3 /* Reserved */ +/* IPMI 2.0, Table 43-1, byte 21[0] Percentage */ +#define SDR_UNIT_PCT_NO 0 +#define SDR_UNIT_PCT_YES 1 + struct { #if WORDS_BIGENDIAN uint8_t analog:2; @@ -394,8 +418,8 @@ struct sdr_record_common_sensor { uint8_t analog:2; #endif struct { - uint8_t base; - uint8_t modifier; + uint8_t base; /* Base unit type code per IPMI 2.0 Table 43-15 */ + uint8_t modifier; /* Modifier unit type code per Table 43-15 */ } ATTRIBUTE_PACKING type; } ATTRIBUTE_PACKING unit; } ATTRIBUTE_PACKING; @@ -833,8 +857,8 @@ void ipmi_sdr_print_sensor_hysteresis(struct sdr_record_common_sensor *sensor, struct sdr_record_full_sensor *full, uint8_t hysteresis_value, const char *hdrstr); -const char *ipmi_sdr_get_unit_string(uint8_t pct, uint8_t type, - uint8_t base, uint8_t modifier); +const char *ipmi_sdr_get_unit_string(bool pct, uint8_t type, + uint8_t base, uint8_t modifier); struct sensor_reading * ipmi_sdr_read_sensor_value(struct ipmi_intf *intf, struct sdr_record_common_sensor *sensor, diff --git a/lib/ipmi_sdr.c b/lib/ipmi_sdr.c index eb40b36..b43765a 100644 --- a/lib/ipmi_sdr.c +++ b/lib/ipmi_sdr.c @@ -68,8 +68,9 @@ static struct sdr_record_list *sdr_list_head = NULL; static struct sdr_record_list *sdr_list_tail = NULL; static struct ipmi_sdr_iterator *sdr_list_itr = NULL; -/* unit description codes (IPMI v1.5 section 37.16) */ -#define UNIT_MAX 0x90 +/* IPMI 2.0 Table 43-15, Sensor Unit Type Codes */ +#define UNIT_TYPE_MAX 92 /* This is the ID of "grams" */ +#define UNIT_TYPE_LONGEST_NAME 19 /* This is the length of "color temp deg K" */ static const char *unit_desc[] = { "unspecified", "degrees C", @@ -161,7 +162,9 @@ static const char *unit_desc[] = { "characters", "error", "correctable error", - "uncorrectable error" + "uncorrectable error", + "fatal error", + "grams" }; /* sensor type codes (IPMI v1.5 table 36.3) @@ -220,35 +223,60 @@ void printf_sdr_usage(); uint16_t ipmi_intf_get_max_response_data_size(struct ipmi_intf * intf); -/* ipmi_sdr_get_unit_string - return units for base/modifier +/** ipmi_sdr_get_unit_string - return units for base/modifier * - * @pct: units are a percentage - * @type: unit type - * @base: base - * @modifier: modifier + * @param[in] pct Indicates that units are a percentage + * @param[in] relation Modifier unit to base unit relation + * (SDR_UNIT_MOD_NONE, SDR_UNIT_MOD_MUL, + * or SDR_UNIT_MOD_DIV) + * @param[in] base The base unit type id + * @param[in] modifier The modifier unit type id * - * returns pointer to static string + * @returns a pointer to static string */ const char * -ipmi_sdr_get_unit_string(uint8_t pct, uint8_t type, uint8_t base, uint8_t modifier) +ipmi_sdr_get_unit_string(bool pct, uint8_t relation, + uint8_t base, uint8_t modifier) { - static char unitstr[16]; + /* + * Twice as long as the longest possible unit name, plus + * four characters for '%' and relation (either ' * ' or '/'), + * plus the terminating null-byte. + */ + static char unitstr[2 * UNIT_TYPE_LONGEST_NAME + 4 + 1]; + /* * By default, if units are supposed to be percent, we will pre-pend * the percent string to the textual representation of the units. */ - char *pctstr = pct ? "% " : ""; - memset(unitstr, 0, sizeof (unitstr)); - switch (type) { - case 2: - snprintf(unitstr, sizeof (unitstr), "%s%s * %s", - pctstr, unit_desc[base], unit_desc[modifier]); + const char *pctstr = pct ? "% " : ""; + const char *basestr; + const char *modstr; + + if (base <= UNIT_TYPE_MAX) { + basestr = unit_desc[base]; + } + else { + basestr = "invalid"; + } + + if (modifier <= UNIT_TYPE_MAX) { + modstr = unit_desc[modifier]; + } + else { + modstr = "invalid"; + } + + switch (relation) { + case SDR_UNIT_MOD_MUL: + snprintf(unitstr, sizeof (unitstr), "%s%s * %s", + pctstr, basestr, modstr); break; - case 1: + case SDR_UNIT_MOD_DIV: snprintf(unitstr, sizeof (unitstr), "%s%s/%s", - pctstr, unit_desc[base], unit_desc[modifier]); + pctstr, basestr, modstr); break; - case 0: + case SDR_UNIT_MOD_NONE: default: /* * Display the text "percent" only when the Base unit is @@ -257,8 +285,8 @@ ipmi_sdr_get_unit_string(uint8_t pct, uint8_t type, uint8_t base, uint8_t modifi if (base == 0 && pct) { snprintf(unitstr, sizeof(unitstr), "percent"); } else { - snprintf(unitstr, sizeof (unitstr), "%s%s", - pctstr, unit_desc[base]); + snprintf(unitstr, sizeof (unitstr), "%s%s", + pctstr, basestr); } break; } -- 2.40.1