I've got a bug report, that 'Raritan Dominion PX' device returns SDR record
with invalid base unit field (returned Sensor Unit Type Code is 94, while IPMI
2.0 describes only 92 units in Table 43-15).
'ipmitool sensor list' then crashes when converting the unit to readable
form - reading from unit_desc gets out of bounds.
This patch tries to protect reading from unit_desc array and also move the
reading to one function instead of it's copies. It prints warning when
processing invalid unit code, I'm not sure if it's the best thing to do - it's
nice to want the user that something is wrong, on the other hand, the user might
get annoyed with the warning when it is using ipmitool regularly, e.g. in
scripts. I don't know how common is HW which sends unkown units.
Signed-off-by: Jan Safranek <jsafr...@redhat.com>
---
lib/ipmi_sdr.c | 59 +++++++++++++++++++++++++++++++----------------------
lib/ipmi_sel.c | 46 +++++++----------------------------------
lib/ipmi_sensor.c | 22 +++-----------------
3 files changed, 45 insertions(+), 82 deletions(-)
diff --git a/lib/ipmi_sdr.c b/lib/ipmi_sdr.c
index 826a34c..040c6fa 100644
--- a/lib/ipmi_sdr.c
+++ b/lib/ipmi_sdr.c
@@ -74,21 +74,45 @@ char *
ipmi_sdr_get_unit_string(uint8_t type, uint8_t base, uint8_t modifier)
{
static char unitstr[16];
+ int unit_count = sizeof(unit_desc) / sizeof(unit_desc[0]);
+ const char *template = NULL, *modifier_str = NULL, *base_str = NULL;
+ int need_modifier;
memset(unitstr, 0, sizeof (unitstr));
switch (type) {
case 2:
- snprintf(unitstr, sizeof (unitstr), "%s * %s",
- unit_desc[base], unit_desc[modifier]);
+ template = "%s * %s";
+ need_modifier = 1;
break;
case 1:
- snprintf(unitstr, sizeof (unitstr), "%s/%s",
- unit_desc[base], unit_desc[modifier]);
+ template = "%s/%s";
+ need_modifier = 1;
break;
case 0:
default:
- snprintf(unitstr, sizeof (unitstr), "%s", unit_desc[base]);
- break;
+ template = "%s";
+ need_modifier = 0;
+ }
+
+ if (base < unit_count)
+ base_str = unit_desc[base];
+ else {
+ lprintf(LOG_WARNING, "Unknown unit: %i", (int) base);
+ base_str = "UNKNOWN";
+ }
+
+ if (need_modifier) {
+ if (modifier < unit_count)
+ modifier_str = unit_desc[modifier];
+ else {
+ lprintf(LOG_WARNING, "Unknown modifier unit: %i",
+ (int) modifier);
+ modifier_str = "UNKNOWN";
+ }
+ snprintf(unitstr, sizeof (unitstr), template,
+ base_str, modifier_str);
+ } else {
+ snprintf(unitstr, sizeof (unitstr), template, base_str);
}
return unitstr;
@@ -1129,7 +1153,7 @@ int
ipmi_sdr_print_sensor_full(struct ipmi_intf *intf,
struct sdr_record_full_sensor *sensor)
{
- char sval[16], unitstr[16], desc[17];
+ char sval[16], *unitstr = NULL, desc[17];
int i = 0, validread = 1, do_unit = 1;
double val = 0.0, creading = 0.0;
struct ipmi_rs *rsp;
@@ -1194,24 +1218,9 @@ ipmi_sdr_print_sensor_full(struct ipmi_intf *intf,
/* determine units with possible modifiers */
if (do_unit && validread) {
- memset(unitstr, 0, sizeof (unitstr));
- switch (sensor->unit.modifier) {
- case 2:
- i += snprintf(unitstr, sizeof (unitstr), "%s * %s",
- unit_desc[sensor->unit.type.base],
- unit_desc[sensor->unit.type.modifier]);
- break;
- case 1:
- i += snprintf(unitstr, sizeof (unitstr), "%s/%s",
- unit_desc[sensor->unit.type.base],
- unit_desc[sensor->unit.type.modifier]);
- break;
- case 0:
- default:
- i += snprintf(unitstr, sizeof (unitstr), "%s",
- unit_desc[sensor->unit.type.base]);
- break;
- }
+ unitstr = ipmi_sdr_get_unit_string(sensor->unit.modifier,
+ sensor->unit.type.base,
+ sensor->unit.type.modifier);
}
/*
diff --git a/lib/ipmi_sel.c b/lib/ipmi_sel.c
index 98757d1..6a10430 100644
--- a/lib/ipmi_sel.c
+++ b/lib/ipmi_sel.c
@@ -1310,25 +1310,10 @@ ipmi_sel_print_extended_entry_verbose(struct ipmi_intf
* intf, struct sel_event_
sdr_convert_sensor_reading(sdr->record.full,
evt->sel_type.standard_type.event_data[1]));
/* determine units with possible modifiers */
- switch (sdr->record.full->unit.modifier) {
- case 2:
- printf(" %s * %s\n",
-
unit_desc[sdr->record.full->unit.type.base],
-
unit_desc[sdr->record.full->unit.type.modifier]);
- break;
- case 1:
- printf(" %s/%s\n",
-
unit_desc[sdr->record.full->unit.type.base],
-
unit_desc[sdr->record.full->unit.type.modifier]);
- break;
- case 0:
- printf(" %s\n",
-
unit_desc[sdr->record.full->unit.type.base]);
- break;
- default:
- printf("\n");
- break;
- }
+ printf(" %s\n", ipmi_sdr_get_unit_string(
+ sdr->record.full->unit.modifier,
+ sdr->record.full->unit.type.base,
+ sdr->record.full->unit.type.modifier));
break;
case 2:
/* oem code in byte 2 */
@@ -1351,25 +1336,10 @@ ipmi_sel_print_extended_entry_verbose(struct ipmi_intf
* intf, struct sel_event_
sdr_convert_sensor_reading(sdr->record.full,
evt->sel_type.standard_type.event_data[2]));
/* determine units with possible modifiers */
- switch (sdr->record.full->unit.modifier) {
- case 2:
- printf(" %s * %s\n",
-
unit_desc[sdr->record.full->unit.type.base],
-
unit_desc[sdr->record.full->unit.type.modifier]);
- break;
- case 1:
- printf(" %s/%s\n",
-
unit_desc[sdr->record.full->unit.type.base],
-
unit_desc[sdr->record.full->unit.type.modifier]);
- break;
- case 0:
- printf(" %s\n",
-
unit_desc[sdr->record.full->unit.type.base]);
- break;
- default:
- printf("\n");
- break;
- }
+ printf(" %s\n", ipmi_sdr_get_unit_string(
+ sdr->record.full->unit.modifier,
+ sdr->record.full->unit.type.base,
+ sdr->record.full->unit.type.modifier));
break;
case 2:
/* OEM code in byte 3 */
diff --git a/lib/ipmi_sensor.c b/lib/ipmi_sensor.c
index f956d69..6dc03d4 100644
--- a/lib/ipmi_sensor.c
+++ b/lib/ipmi_sensor.c
@@ -227,7 +227,7 @@ static int
ipmi_sensor_print_full_analog(struct ipmi_intf *intf,
struct sdr_record_full_sensor *sensor)
{
- char unitstr[16], id[17];
+ char *unitstr, id[17];
int i = 0, validread = 1, thresh_available = 1;
double val = 0.0;
struct ipmi_rs *rsp;
@@ -275,24 +275,8 @@ ipmi_sensor_print_full_analog(struct ipmi_intf *intf,
/*
* Figure out units
*/
- memset(unitstr, 0, sizeof (unitstr));
- switch (sensor->unit.modifier) {
- case 2:
- i += snprintf(unitstr, sizeof (unitstr), "%s * %s",
- unit_desc[sensor->unit.type.base],
- unit_desc[sensor->unit.type.modifier]);
- break;
- case 1:
- i += snprintf(unitstr, sizeof (unitstr), "%s/%s",
- unit_desc[sensor->unit.type.base],
- unit_desc[sensor->unit.type.modifier]);
- break;
- case 0:
- default:
- i += snprintf(unitstr, sizeof (unitstr), "%s",
- unit_desc[sensor->unit.type.base]);
- break;
- }
+ unitstr = ipmi_sdr_get_unit_string(sensor->unit.modifier,
+ sensor->unit.type.base, sensor->unit.type.modifier);
/*
* Get sensor thresholds
------------------------------------------------------------------------------
Throughout its 18-year history, RSA Conference consistently attracts the
world's best and brightest in the field, creating opportunities for Conference
attendees to learn about information security's most important issues through
interactions with peers, luminaries and emerging and established companies.
http://p.sf.net/sfu/rsaconf-dev2dev
_______________________________________________
Ipmitool-devel mailing list
Ipmitool-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
