ipmitool / test Goto Github PK

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/208

There is one big help text in 'lib/ipmi_lanp.c' and then small portions of this help text duplicated in ipmi_lan_set() as well. And they're duplicated even more than once.
This is something that should be reworked.

Suggested fix is to break help text into smaller functions and then call them on demand or all of them if complete help text has to be printed out.

Reported by: Unknown User (*anonymous)
Original Ticket: ipmitool/bugs/275

I've noticed on our systems that ipmievd does not print the correct logging message for drive bay sensors. The drive bays are Compact Sensor (type 2) in the SDR, and indicate that this is a shared record (Share Count = 8).

The SDR has a single entry for drive slots, sensor #80 with share count = 8. The next sensor number in the SDR is #90. When a drive is pulled/inserted, the SEL will contain entries for sensor numbers #8x.

ipmievd will only print correctly for sensor #80 (Slot 0). I've traced this down to ipmi_sdr_find_sdr_by_numtype in lib/ipmi_sdr.c. This function only searches for an exact sensor number to SDR match.

I'm proposing to change ipmi_sdr.c to support reverse lookup of sensor range for shared compact sensors in ipmi_sdr_find_sdr_by_numtype().

Signed-off-by: Jordan Hargrave <[email protected]>

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/134

Function ipmi_fru_read_to_bin() in 'lib/ipmi_fru.c' needs a return value. Since FS operation is involved and since this function is called straight away, return value is a must. As of now, there is no way to determine whether function ended with success or not.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/231

There are several cases of ``if (verbose == LOG_DEBUG)'' and I'm wondering whether this intentionaly or not. Because it means user would have to give '-v' times seven to see debug output.

Reported by: ledva
Original Ticket: ipmitool/bugs/213

always false. removing unreachable code.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/234

Return value of ipmi_ek_display_board_info_area() should be checked by caller as this function will return (-1) on error.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/147

There is not much to say as subject says it all. % ipmitool fru get; is not documented anywhere - not in help output, not in man page. Its only evidence exists only in the code.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/227

The following help output could and should be improved, resp. prefix "flags" with "param #", so it's obvious what has to be supplied to 'chassis bootparam get'.

\# ipmitool chassis bootparam get 
bootparam get <param \#>
bootparam set bootflag <flag>
force\_pxe   : Force PXE boot
force\_disk  : Force boot from default Hard-drive
force\_safe  : Force boot from default Hard-drive, request Safe Mode
force\_diag  : Force boot from Diagnostic Partition
force\_cdrom : Force boot from CD/DVD
force\_bios  : Force boot into BIOS Setup

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/250

It should/has to be documented ipmitool leaks memory(ID#3613605) when multiple '-Y' and/or '-a' is/are specified. This can't be fixed without "breaking backwards compatibility"(whatever it means).
Anyway, the only way how to fix this is to limit how many '-Y'/'-a' can be specified/accepted on command line. Perhaps in 1.9 or 2.0? But until it's fixed, memory leak should be documented as a known issue.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/270

Replace scanf() calls with something more appropriate and safe. Until replaced, this is an open security hole with nice and lean buffer overflow.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/132

Debug messages will end up on STDOUT if turned on. This doesn't look right. This is caused by use of printf() rather than using lprintf(LOG_DEBUG, ...);.
This has to be fixed.

Reported by: wferi
Original Ticket: ipmitool/bugs/362

Apparently the LOCAL4 syslog facility is hardwired in lib/log.c.
Please include a command line option for changing it.
Actually, I may even implement this if there's consensus on how it should be done, so that my work gets merged.

Reported by: Rob Swindell
Original Ticket: ipmitool/bugs/142

(v1.8.12) ipmitool does not validate trailing checksum of IPMI responses (the checksum following the response data). E.g. if the MC always responds with a trailling checksum value of 0x00, ipmitool doesn't complain.

Reported by: ledva
Original Ticket: ipmitool/bugs/215

size can't be negative - dead code

the the "-" result in the unsigned storage is never negative

Reported by: Rob Swindell
Original Ticket: ipmitool/bugs/139

Ater creating an RMCP+ session with a succesfully negotiated cipher-suite containing an integrity and/or confidentiality (encryption) algorithm (e.g. "-C2 or -C3"), if the management controller responds with non-authenticated/integiryt-protected or non-encrypted IPMI/RMCP+ responses, ipmitool accepts the response as valid.

This is a security flaw in ipmitool but can only be observed with a non-conforming management controller or a malicious man-in-the-middle.

This is an old ipmitool bug, but still exists in v1.8.12.

Reported by: Rob Swindell
Original Ticket: ipmitool/bugs/140

(v1.8.12) There is no mechanism for clearing the Asset Tag. The command 'ipmitool dcmi set_asset_tag ""' doesn't actually do anything". The 'ipmitool dcmi set_mc_id_string' command does not have this problem.

It stands to reason that ipmitool should provide a mechanism to clear (set to 0-length) the DCMI asset tag.

ipmitool sdr -v

Sensor ID : Temp (0x1)
Entity ID : 3.1 (Processor)
Sensor Type (Analog) : Temperature
Sensor Reading : -69 (+/- 1) degrees C
Status : ok
Nominal Reading : 50.000
Normal Minimum : 11.000
Normal Maximum : 69.000
Upper critical : 90.000
Upper non-critical : 85.000
Positive Hysteresis : 1.000
Negative Hysteresis : 1.000
Minimum sensor range : Unspecified
Maximum sensor range : Unspecified
Event Message Control : Per-threshold
Readable Thresholds : unc ucr
Settable Thresholds :
Threshold Read Mask : unc ucr
Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

ipmitool sensor -v

Sensor ID : Temp (0x1)
Entity ID : 3.1
Sensor Type (Analog) : Temperature
Sensor Reading : READ ERROR: Device Not Present

Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

So sdr is reading the entity while sensor doesn't.

Reported by: costake

Original Ticket: "ipmitool/bugs/36":https://sourceforge.net/p/ipmitool/bugs/36

Reported by: Duncan Idaho
Original Ticket: ipmitool/bugs/103

There are couple things in 'lib/ipmi_ekanalyzer.c' that would use re-work. I've noticed only print-outs.

Why???

static void
ipmi\_ekanalyzer\_usage\( void \)
\{
char \* help\_message =
"Ekeying analyzer tool version 1.00                                        \n\
ekanalyzer Commands:                                                       \n\
print    \[carrier | power | all\] <oc=filename1> <b1=filename2>...    \n\
frushow  <b2=filename>                                               \n\
summary  \[match | unmatch | all\] <oc=filename1> <b1=filename2>...    \n\
";
printf\("%s",help\_message\);
fflush\(stdout\);
\}

Typo _argumentS_ + Repetitive -> create function:

printf\("Too few argument\! \n"\);
if \( strcmp\(argv\[argument\_offset\], "print"\) == 0 \)\{
lprintf\(LOG\_ERR, "   ekanalyzer print \[carrier/power/all\]"
" <xx=frufile> <xx=frufile> \[xx=frufile\]"
\);
\}
else\{
lprintf\(LOG\_ERR, "   ekanalyzer summary \[match/ unmatch/ all\]"
" <xx=frufile> <xx=frufile> \[xx=frufile\]"
\);
\}

Should be at STDERR:

printf\("Too few argument\! \n"\);
\[...\]
printf\("Invalid option '%s'\n", argv\[argument\_offset\]\);
\[...\]
if \( \!found\_flag \)\{
printf\("No carrier file has been found\n"\);
return\_value = ERROR\_STATUS;
\}
\[...\]
if \( amc\_file == FALSE \)\{
printf\("\nNo AMC FRU file is provided --->" \
" No possible ekeying match\!\n"\);
return\_value = ERROR\_STATUS;
\}
\[...\]
if \( \!oc\_file \)\{
printf\("\nNo Carrier FRU file is provided" \
" ---> No possible ekeying match\!\n"\);
return\_value = ERROR\_STATUS;
\}
\[...\]
else\{
printf\("No amc record is found\!\n"\);
\}
\[...\]
if \( record == NULL \)\{
printf\("NO Carrier p2p connectivity \!\n"\);
return\_status = ERROR\_STATUS;
\}

Perhaps there is more. There is some debug information passed straight to STDOUT; I think it should go to STDERR.

Reported by: ledva
Original Ticket: ipmitool/bugs/154

patch candidate in the bz. please comment.
https://bugzilla.redhat.com/show\_bug.cgi?id=857106
this is rfc on proposed fix to sol keepalives and related findings.

the current code contains comment that payload processor reboot may cause unwanted termination.
right now it does not tear down the session as long as there is no data to send.
when there are no commands pending then no command may fail and the session will persist.
the getdeviceid method is prone to hanging around even after interfering deactivation.
should not the default session live detection method be changed to activation (and quit after 2nd successful) or is there better method of detecting activity?

the retry and timeout are values used at the lanplus session command level
the solretry and soltime values are used for the live check while there is no valid command pending.

also the retry and timeout defaults but that's probably an enhancement.
keep it fixed or get the remote parameters using the info and have the defaults mimic the remote defaults?

also does anyone see why is there timeout = 1 setting?

and using the

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/228

PARAM in 'chassis bootparam get PARAM' should be limited/locked to range <0..6> as per IPMIv2 specification, PDF, p. 392. There is no reason why ipmitool should accept other values.

Reported by: Duncan Idaho
Original Ticket: ipmitool/bugs/124

ipmi_sdr_add_record() in 'lib/ipmi_sdradd.c' has no response and no check whether SDR add was successful or not which seems to be odd. There should be response as well as check of ccode; both are missing at the moment.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/131

(Some) error messages are printed out on STDOUT instead STDERR. This is because printf() is used, although lprintf() should be used for all printing in ipmitool. This has to be fixed.

Reported by: Unknown User (*anonymous)
Original Ticket: ipmitool/bugs/351

The requested maximum privilege level in RAKP 1 should be depended on the open session response.
The byte 3 of the open session response indicates the "maximum" privilege level allowed for this session based on the cyphite suite.
It means the requested privilege level in RAKP 1 (byte 25) should not higher than the level in the open session response.
In the current implement of ipmitool 1.8.9, the administrator command can be executed in the user level session if requested level in RAKP 1 is ADMIN but the allowed maximum level is USER in open session response.
It is conflict betwenn IPMI definition.
According to the description of byte 25 bit 4 in RAKP 1, BMC will select the minimum level between "requested level", channel level and user level.
ex: ipmitool -I lanplus -H x.x.x.x -C 0 -U root -P root chassis power on
This command can be success executed even if the privilege level of cyphite suite '0' is USER and the level of user 'root' is ADMIN.

Reported by: ledva
Original Ticket: ipmitool/bugs/281

https://sourceforge.net/p/ipmitool/mailman/message/31657636/

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/156

The following should return RC > 0 on error:

if \(rsp->data\[0\] \!= 0x00\) \{
lprintf\(LOG\_ERR, "Error activation/deactivation of FRU."\);
\}

return 0;
\}

Currently, the sel add from file command does not work for me.

When adding entries to SEL (from file), the tool must first reserve the SEL, and then use the returned reservation code in the sel add command.

Reported by: mdv

Original Ticket: "ipmitool/bugs/37":https://sourceforge.net/p/ipmitool/bugs/37

Reported by: Unknown User (*anonymous)
Original Ticket: ipmitool/bugs/352

Raise session privilege level after the session established in revision 1.41 is not necessary for IPMI 2.0 session, but necessary in 1.5.
According to the IPMI 2.0 session establish flow, BMC creates the session with minimum privilege of requested maximum privilege level in RAKP 1, user maximum privilege and channel maximum privilege.
It means the privilege level is the maximum "allowed" level in the established session.
This command cannot be used to set a privilege level higher than the lowest of the privilege level of the user and the privilege limit of the channel.

Reported by: Unknown User (*anonymous)
Original Ticket: ipmitool/bugs/48

Execute set sensor threshold command to change only one of the thresholds of a sensor. As per IPMI specification, the comparison of thresholds must be in the order LNR < LC < LNC < sensor's reading < UNC < UC < UNR.
So only after comparing the sensor's readings as above the status of the sensor is determined. But with the ipmitool 1.8.11, the status of the sensor changes even if one of the threshold changes.

Example:
1.[root@localhost ~]# ipmitool -H device_ip -U username -P password raw 4 0x26 1 0x20 1 2 3 0xf0 0xf1 4

2. [root@localhost ~]# ipmitool -H device_ip -U username -P password sdr
voltage_33SBV | 3.32 Volts | nr
Voltage_33V | 3.32 Volts | ok
voltage_5V | 4.99 Volts | ok
Voltage_VCC | 2.30 Volts | ok
Fan_Presence | 0x01 | ok
Temp_1 | 26 degrees C | ok
Temp_2 | 26 degrees C | ok
Fan_1 | 5000 RPM | ok
Fan_2 | 5000 RPM | ok
Fan_3 | 5000 RPM | ok
Chassis_Intr | 0x01 | ok

As you can see from the step1, the byte 0x20 indicates that I'm going to set only upper non recoverable threshold and not upper critical and upper non critical values. Still sdr command shows the status of the sensor as Non recoverable (nr) even though UC and UNC values are higher than sensor readings. This applies to all combinations of threholds.

ipmitool sdr -v

Sensor ID : Temp (0x1)
Entity ID : 3.1 (Processor)
Sensor Type (Analog) : Temperature
Sensor Reading : -69 (+/- 1) degrees C
Status : ok
Nominal Reading : 50.000
Normal Minimum : 11.000
Normal Maximum : 69.000
Upper critical : 90.000
Upper non-critical : 85.000
Positive Hysteresis : 1.000
Negative Hysteresis : 1.000
Minimum sensor range : Unspecified
Maximum sensor range : Unspecified
Event Message Control : Per-threshold
Readable Thresholds : unc ucr
Settable Thresholds :
Threshold Read Mask : unc ucr
Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

ipmitool sensor -v

Sensor ID : Temp (0x1)
Entity ID : 3.1
Sensor Type (Analog) : Temperature
Sensor Reading : READ ERROR: Device Not Present

Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

So sdr is reading the entity while sensor doesn't.

Reported by: costake

Original Ticket: "ipmitool/bugs/36":https://sourceforge.net/p/ipmitool/bugs/36

Reported by: Nirmal Kumar
Original Ticket: ipmitool/bugs/52

when the sensor is going high i.e upper critical or lower critical or whatever state, it is just showing that state not whether upper or lower, when we put sensor list

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/148

Command % ipmitool fru internaluse; is not documented in man page. However, it's documented in % ipmitool fru help;, resp. % ipmitool fru internaluse help;.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/202

Code to blame in 'lib/ipmi_chassis.c':

case 4:
\{
printf\(   " Boot Info Acknowledge :\n"\);
if\(\(rsp->data\[3\]&0x1f\) \!= 0\)
\{
if\(\(rsp->data\[3\]&0x10\) == 0x10\)
printf\("    - OEM has handled boot info\n"\);
if\(\(rsp->data\[3\]&0x08\) == 0x08\)
printf\("    - SMS has handled boot info\n"\);
if\(\(rsp->data\[3\]&0x04\) == 0x04\)
printf\("    - OS // service partition has handled boot info\n"\);
if\(\(rsp->data\[3\]&0x02\) == 0x02\)
printf\("    - OS Loader has handled boot info\n"\);
if\(\(rsp->data\[3\]&0x01\) == 0x01\)
printf\("    - BIOS/POST has handled boot info\n"\);
\}
else
\{
printf\("     No flag set\n"\);
\}
\}

IPMI v2 specification p.391 in PDF:

\[7\] -    reserved. Write as 1b. Ignore on read. 
\[6\] -    reserved. Write as 1b. Ignore on read. 
\[5\] -    reserved. Write as 1b. Ignore on read. 
\[4\] -    0b = OEM has handled boot info. 
\[3\] -    0b = SMS has handled boot info. 
\[2\] -    0b = OS / service partition has handled boot info. 
\[1\] -    0b = OS Loader has handled boot info. 
\[0\] -    0b = BIOS/POST has handled boot info. 

Code in 'lib/ipmi_chassis.c' would be true if ``rsp->data[3]'' got inverted. But it doesn't, so the code in 'lib/ipmi_chassis.c' is invalid/doesn't work.
It also would be worth of checking the whole function and do a re-make/re-write.

Reported by: ledva
Original Ticket: ipmitool/bugs/214

become dead code with fixed leaks

Reported by: Rob Swindell
Original Ticket: ipmitool/bugs/243

ipmitool "dcmi power reading" command does not support IPMI "Special Timestamp Values" (IPMI 2.0 section 37.1). Timestamp values of 0xffffffff and between 0x00000000 between 0x20000000 should be treated "specially" and *not* parsed
/displayed as a "seconds since epoch" value.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/206

\-	if \(argc == 0 || strncmp\(argv\[0\], "help", 4\) == 0\) \{
\-		ipmi\_event\_usage\(\);
\-		return 0;
\-	\}

+ if (argc == 0) {
+ lprintf(LOG_ERR, "Not enough parameters given.");
+ ipmi_event_usage();
+ return (-1);
+ } else if (strncmp(argv[0], "help", 4) == 0) {
+ ipmi_event_usage();
+ return 0;
+ }

This is also wrong:

if (strncmp(argv[0], "file", 4) == 0) {
if (argc < 2) {
ipmi_event_usage();
return 0;
}
return ipmi_event_fromfile(intf, argv[1]);
}

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/133

The following code looks a bit mixed up, doesn't it?

/\* \_\_ipmi\_fru\_print  -  Do actual work to print a FRU by its ID
\*
\* @intf:   ipmi interface
\* @id:     fru id
\*
\* returns -1 on error
\* returns 0 if successful
\* returns 1 if device not present
\*/
static int
\_\_ipmi\_fru\_print\(struct ipmi\_intf \* intf, uint8\_t id\)
\{
rsp = intf->sendrecv\(intf, &req\);
if \(rsp == NULL\) \{
printf\(" Device not present \(No Response\)\n"\);
return -1;
\}
if \(rsp->ccode > 0\) \{
printf\(" Device not present \(%s\)\n",
val2str\(rsp->ccode, completion\_code\_vals\)\);
return -1;
\}
\[...\]
rsp = intf->sendrecv\(intf, &req\);
if \(rsp == NULL\) \{
printf\(" Device not present \(No Response\)\n"\);
return 1;
\}
if \(rsp->ccode > 0\) \{
printf\(" Device not present \(%s\)\n",
val2str\(rsp->ccode, completion\_code\_vals\)\);
return 1;
\}
\[...\]
\}

Reported by: costake
Original Ticket: ipmitool/bugs/36

ipmitool sdr -v

Sensor ID : Temp (0x1)
Entity ID : 3.1 (Processor)
Sensor Type (Analog) : Temperature
Sensor Reading : -69 (+/- 1) degrees C
Status : ok
Nominal Reading : 50.000
Normal Minimum : 11.000
Normal Maximum : 69.000
Upper critical : 90.000
Upper non-critical : 85.000
Positive Hysteresis : 1.000
Negative Hysteresis : 1.000
Minimum sensor range : Unspecified
Maximum sensor range : Unspecified
Event Message Control : Per-threshold
Readable Thresholds : unc ucr
Settable Thresholds :
Threshold Read Mask : unc ucr
Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

ipmitool sensor -v

Sensor ID : Temp (0x1)
Entity ID : 3.1
Sensor Type (Analog) : Temperature
Sensor Reading : READ ERROR: Device Not Present

Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

So sdr is reading the entity while sensor doesn't.

ipmitool sdr -v

Sensor ID : Temp (0x1)
Entity ID : 3.1 (Processor)
Sensor Type (Analog) : Temperature
Sensor Reading : -69 (+/- 1) degrees C
Status : ok
Nominal Reading : 50.000
Normal Minimum : 11.000
Normal Maximum : 69.000
Upper critical : 90.000
Upper non-critical : 85.000
Positive Hysteresis : 1.000
Negative Hysteresis : 1.000
Minimum sensor range : Unspecified
Maximum sensor range : Unspecified
Event Message Control : Per-threshold
Readable Thresholds : unc ucr
Settable Thresholds :
Threshold Read Mask : unc ucr
Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

ipmitool sensor -v

Sensor ID : Temp (0x1)
Entity ID : 3.1
Sensor Type (Analog) : Temperature
Sensor Reading : READ ERROR: Device Not Present

Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

So sdr is reading the entity while sensor doesn't.

Reported by: costake

Original Ticket: "ipmitool/bugs/36":https://sourceforge.net/p/ipmitool/bugs/36

Reported by: Unknown User (*anonymous)
Original Ticket: ipmitool/bugs/39

ipmi_lan_poll_recv assumes that the two "Send Message" responses will be consecutive. If this is not the case, the second response will be dropped. This was observed when a MicroTCA Shelf Manager sent a "Set Event Receiver" to ipmitool, assuming that it was a Carrier Manager. The "Set Event Receiver" would often appear between the "Send Message" ack and the bridged command response.

I have looked at the code, but I don't see an easy way to fix this.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/135

Function ipmi_fru_write_from_bin() in 'lib/ipmi_fru.c' needs a return value. Since FS operation is involved and since this function is called straight away, return value is a must. As of now, there is no way to determine whether function ended with success or not.

Reported by: costake
Original Ticket: ipmitool/bugs/36

ipmitool sdr -v

Sensor ID : Temp (0x1)
Entity ID : 3.1 (Processor)
Sensor Type (Analog) : Temperature
Sensor Reading : -69 (+/- 1) degrees C
Status : ok
Nominal Reading : 50.000
Normal Minimum : 11.000
Normal Maximum : 69.000
Upper critical : 90.000
Upper non-critical : 85.000
Positive Hysteresis : 1.000
Negative Hysteresis : 1.000
Minimum sensor range : Unspecified
Maximum sensor range : Unspecified
Event Message Control : Per-threshold
Readable Thresholds : unc ucr
Settable Thresholds :
Threshold Read Mask : unc ucr
Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

ipmitool sensor -v

Sensor ID : Temp (0x1)
Entity ID : 3.1
Sensor Type (Analog) : Temperature
Sensor Reading : READ ERROR: Device Not Present

Event Status : Event Messages Disabled
Assertion Events :
Event Enable : Event Messages Disabled
Assertions Enabled :

So sdr is reading the entity while sensor doesn't.

Reported by: Roger Hung
Original Ticket: ipmitool/bugs/288

In IPMI Spec table 20-10, GUID bytes are transmitted in ‘network order’ (most-significant byte first) starting with the time low field. But ipmitool display method doesn't match this table format. Also ipmitool uses int32 and int16 to define timestamp (time_low,time_mid,time_hi_and_version) impacted by little endian.

In ipmi_mc.c:519 (ipmitool-1.8.13):
/* Kipp - changed order of last field (node) to follow specification */
printf("System GUID : %08x-%04x-%04x-%04x-%02x%02x%02x%02x%02x%02x\n",
guid.time_low, guid.time_mid, guid.time_hi_and_version,
guid.clock_seq_hi_variant << 8 | guid.clock_seq_low,
guid.node[0], guid.node[1], guid.node[2],
guid.node[3], guid.node[4], guid.node[5]);

Reported by: ledva
Original Ticket: ipmitool/bugs/216

either the patch or maybe modify to tristate return value and the associated code?

Reported by: jdelvare
Original Ticket: ipmitool/bugs/350

When IPMI records have ID strings larger than 16 bytes, we have to make sure we only read 16 bytes, because the structures used only have room for that many bytes, and so does the desc buffer.

Note: I made the changes minimal because I am not familiar with the code. But I am really curious why the structures only reserve only 16 bytes for id_string when the IPMI specification says the maximum valid length is 30.

Reported by: Al Chu
Original Ticket: ipmitool/bugs/47

I believe the output of the sensor bitmasks is incorrect. data[2] represents state offsets 0-7 and data[3] represents state offsets 8-14. So semantically, I believe the output should be:

printf("0x%02x%02x", rsp->data[3], rsp->data[2]);

instead of what is currently done, which is:

printf("| 0x%-8x | %-10s | 0x%02x%02x",
val,
unitstr, rsp->data[2], rsp->data[3]);

It's possible that the original output wanted to output event bitmasks 0-14 in order left to right, but it's actually outputting "7-0,reserved(ie. bit 15),14-8".

Francois Isabelle responded on the mailing list:

"I personally agree but I'm afraid our users are used to this reversed notation."

Fair enough. But it should be documented as a "bug: will not fix" or something like that.

Reported by: Rob Swindell
Original Ticket: ipmitool/bugs/145

1. "dcmi thermalpolicy" only shows "Get <entityID> <instanceID>" as valid argument in help output, when in fact, "Set" is also available.

2. "dcmi thermalpolicy set <entityID> <instanceID>" shows another help screen with all the required parameters and values. This should be shown in the "dcmi thermalpolicy" help output.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/271

Review all sscanf() calls and add return value/errno checks. The reason is security, of course.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/203

Code to blame follows. The code is wrong as has been discussed on mailing list:

case 8:	/\* SR2300, redundant, PS1 & PS2 present \*/
if \(verbose\) \{
printf\("Power Redundancy       : No\n"\);
printf\("Power Supply 2 Sensor  : %x\n",
oem->data\[8\]\);
\} else if \(csv\_output\) \{
printf\("Power Redundancy,PS@%02xh,nr\n",
oem->data\[8\]\);
\} else \{
printf
\("Power Redundancy | PS@%02xh            | nr\n",
oem->data\[8\]\);
\}
case 9:	/\* SR2300, non-redundant, PSx present \*/
if \(verbose\) \{
printf\("Power Redundancy       : Yes\n"\);
printf\("Power Supply Sensor    : %x\n",
oem->data\[7\]\);
printf\("Power Supply Sensor    : %x\n",
oem->data\[8\]\);
\} else if \(csv\_output\) \{
printf
\("Power Redundancy,PS@%02xh + PS@%02xh,ok\n",
oem->data\[7\], oem->data\[8\]\);
\} else \{
printf
\("Power Redundancy | PS@%02xh + PS@%02xh   | ok\n",
oem->data\[7\], oem->data\[8\]\);
\}
break;
\}
if \(verbose\)
printf\("\n"\);
break;

Follows copy-paste from Liebig Holger:

\[Liebig, Holger\]
To answer this myself: in http://download.intel.com/support/motherboards/server/sb/imm\_tps\_12.pdf page 200 is a list and description of Intel OEM SDRs including the Power Unit Map SDR type. The same information can be found in a dusty yellow cover document from 2004 for the ESB2 BMC from Intel. A real world SDR including such an OEM SDR can be found on http://communities.intel.com/thread/11855?start=15&tstart=0, so this seems to be a common OEM SDR type for Intel systems.

Based on this information:
\- a break for the case 8 is indeed missing
\- case 8 should output the PSU sensor number from oem->data\[7\] instead of oem->data\[8\] since only one PSU sensor number is available.
\- the comments for case 8 and case 9 are misleading and should be swapped
\- references to specific system codes \(SR2300/SR1300\) could be removed
\- the "redundancy available" information should evaluate the "Power Supplies Required" field of the SDR and not simply rely on the number of PSU sensors being > 1. The sample in table 107 on page 200 requires 2 PSU for the system to work and has 3 PSU sensors \(2+1 redundancy\)

Holger

Reported by: Duncan Idaho
Original Ticket: ipmitool/bugs/119

It seems functions in 'lib/ipmi_gendev.c' do return 0 even on error. Some example follows:

ipmi\_gendev\_read\_file\(...\) \{
int rc = 0;
\[...\]
if\(counter == \(eeprom\_info.size\)\)
\{
printf\("\r%%100 percent completed\n"\);
\}
else
\{
printf\("\rError: %i percent completed, read not completed \n", percentCompleted\);
\}

fclose\(fp\);
\}
\}
else
\{
lprintf\(LOG\_ERR, "The selected generic device is not an eeprom"\);
\}

return rc;
\}

Check 'lib/ipmi_gendev.c' for similar cases. There is more than just one.

Reported by: Zdenek Styblik
Original Ticket: ipmitool/bugs/360

This is due to the fact of possible race condition in fopen(). Also, ipmi_open_file() could double if there was a way how to pass mode or whatever. We'll see, but scattered use of fopen() and almost no use of ipmi_open_file(), which does some reasonable(?) FS checks, seems wrong.