sysutils/ipmitool: Apply multiples patches

Integrates several backported patches from different origins, with some
aligning with those chosen for the Ubuntu ipmitool package while awaiting
upstream inclusion.
These patches address:
- [open interface] Eliminate buffer overrun
- [dcmi] Update parameters to read temperature sensors
- [fru] Update the fru section offset only when they exist
- [fru] Adjust the fru section by their offset order
- [lan] Fix error response from Unsupported Parameter lookup
- [lan] Fix lan print fails on unsupported parameters
- [sdr] Fix sensor reading
- [sdr] Fix command ipmitool sdr type with raw values

PR:		282617
Approved by:	zi (maintainer)
Sponsored by:	Netflix
This commit is contained in:
Olivier Cochard
2025-05-14 19:11:33 +02:00
parent 3c0c45b9a1
commit 260b5ad90f
8 changed files with 566 additions and 1 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
PORTNAME= ipmitool
PORTVERSION= 1.8.19
PORTREVISION= 2
PORTREVISION= 3
CATEGORIES= sysutils
MASTER_SITES= https://codeberg.org/IPMITool/${PORTNAME}/archive/:ipmi \
LOCAL/zi/:iana
@@ -0,0 +1,33 @@
From b4bc5c335159b1c272e06dba98e2916e3ecc0462 Mon Sep 17 00:00:00 2001
From: Howitzer105mm <howitzer105mm@noreply.codeberg.org>
Date: Tue, 26 Mar 2024 11:28:16 +0000
Subject: [PATCH] open: Eliminate buffer overrun (#24)
clangd reports a buffer overrun issue in `open` interface.
The sprintf() used to fill ipmi_devfs2 requires 17 bytes to store the
null terminated string. The character buffer is only 16 bytes in
length.
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
Reviewed-on: https://codeberg.org/IPMITool/ipmitool/pulls/24
Reviewed-by: Alexander Amelkin <alexander@amelkin.msk.ru>
Co-authored-by: Howitzer105mm <howitzer105mm@noreply.codeberg.org>
Co-committed-by: Howitzer105mm <howitzer105mm@noreply.codeberg.org>
---
src/plugins/open/open.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git src/plugins/open/open.c src/plugins/open/open.c
index 39c8ea2..88cb6c3 100644
--- src/plugins/open/open.c
+++ src/plugins/open/open.c
@@ -94,7 +94,7 @@ ipmi_openipmi_open(struct ipmi_intf *intf)
{
char ipmi_dev[16];
char ipmi_devfs[16];
- char ipmi_devfs2[16];
+ char ipmi_devfs2[17];
int devnum = 0;
devnum = intf->devnum;
+57
View File
@@ -0,0 +1,57 @@
From ab5ce5baff097ebb6e2a17a171858be213ee68d3 Mon Sep 17 00:00:00 2001
From: Thang Tran <thuutran@amperecomputing.com>
Date: Tue, 11 Jul 2023 17:27:12 +0700
Subject: [PATCH] dcmi: update parameters to read temperature sensors
Issue:
When the system has number of CPU temperature sensors more than 8,
"ipmitool dcmi get_temp_reading" can not show all of sensors.
Root cause:
To request to read multiple sensors for each message, ipmitool has to
send "Get Temperature Readings" command with the "Entity Instance" always
0 and the "Entity Instance Start" as the offset. But the current code is
sending "Entity Instance" is offset and "Entity Instance Start" always is
0. It makes ipmitool only read 1 sensor each time. Besides that, the
"Entity Instance Start" value starts from 1 (not 0), therefore, the
initialization has to be set to 1.
Solution:
This commit corrects the order of parameters and the initialization of
"Entity Instance Start" byte.
Resolves ipmitool/ipmitool#3
Tested:
1. Update BMC software to support 24 CPU temperature sensors
2. Request to read the temperature sensors
$ipmitool dcmi get_temp_reading
3. Display full 24 CPU temperature sensors.
Signed-off-by: Thang Tran <thuutran@amperecomputing.com>
---
lib/ipmi_dcmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git lib/ipmi_dcmi.c lib/ipmi_dcmi.c
index 8ed87a9..8cf6d66 100644
--- lib/ipmi_dcmi.c
+++ lib/ipmi_dcmi.c
@@ -1595,7 +1595,7 @@ ipmi_dcmi_prnt_get_temp_readings(struct ipmi_intf * intf)
continue;
}
/* Total number of available instances for the Entity ID */
- offset = 0;
+ offset = 1;
tota_inst = rsp->data[1];
while (tota_inst > 0) {
get_inst = ((tota_inst / DCMI_MAX_BYTE_TEMP_READ_SIZE) ?
@@ -1603,7 +1603,7 @@ ipmi_dcmi_prnt_get_temp_readings(struct ipmi_intf * intf)
(tota_inst % DCMI_MAX_BYTE_TEMP_READ_SIZE));
rsp = ipmi_dcmi_get_temp_readings(intf,
dcmi_temp_read_vals[i].val,
- offset, 0);
+ 0, offset);
if (chk_rsp(rsp)) {
continue;
}
+296
View File
@@ -0,0 +1,296 @@
From 81011685ea5e8897f8c0971eca5feb93c6880f09 Mon Sep 17 00:00:00 2001
From: Andrew Liao <andrew8325@outlook.com>
Date: Fri, 23 Sep 2022 10:11:04 +0800
Subject: [PATCH 1/2] fru: Update the fru section offset only when they exist
(offset is not 0)
---
lib/ipmi_fru.c | 52 ++++++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 23 deletions(-)
diff --git lib/ipmi_fru.c lib/ipmi_fru.c
index 3d1d8a1a..a994f3cf 100644
--- lib/ipmi_fru.c
+++ lib/ipmi_fru.c
@@ -5052,35 +5052,41 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
/* Chassis type field */
if (f_type == 'c' )
{
- printf("Moving Section Chassis, from %i to %i\n",
- ((header.offset.board) * 8),
- ((header.offset.board + change_size_by_8) * 8)
- );
- memcpy(
- (fru_data_new + ((header.offset.board + change_size_by_8) * 8)),
- (fru_data_old + (header.offset.board) * 8),
- board_len
- );
- header.offset.board += change_size_by_8;
+ if (header.offset.board != 0) {
+ printf("Moving Section Chassis, from %i to %i\n",
+ ((header.offset.board) * 8),
+ ((header.offset.board + change_size_by_8) * 8)
+ );
+ memcpy(
+ (fru_data_new + ((header.offset.board + change_size_by_8) * 8)),
+ (fru_data_old + (header.offset.board) * 8),
+ board_len
+ );
+ header.offset.board += change_size_by_8;
+ }
}
/* Board type field */
if ((f_type == 'c' ) || (f_type == 'b' ))
{
- printf("Moving Section Product, from %i to %i\n",
- ((header.offset.product) * 8),
- ((header.offset.product + change_size_by_8) * 8)
- );
- memcpy(
- (fru_data_new + ((header.offset.product + change_size_by_8) * 8)),
- (fru_data_old + (header.offset.product) * 8),
- product_len
- );
- header.offset.product += change_size_by_8;
+ if (header.offset.product != 0) {
+ printf("Moving Section Product, from %i to %i\n",
+ ((header.offset.product) * 8),
+ ((header.offset.product + change_size_by_8) * 8)
+ );
+ memcpy(
+ (fru_data_new + ((header.offset.product + change_size_by_8) * 8)),
+ (fru_data_old + (header.offset.product) * 8),
+ product_len
+ );
+ header.offset.product += change_size_by_8;
+ }
}
- if ((f_type == 'c' ) || (f_type == 'b' ) || (f_type == 'p' )) {
- printf("Change multi offset from %d to %d\n", header.offset.multi, header.offset.multi + change_size_by_8);
- header.offset.multi += change_size_by_8;
+ if (header.offset.multi != 0) {
+ if ((f_type == 'c' ) || (f_type == 'b' ) || (f_type == 'p' )) {
+ printf("Change multi offset from %d to %d\n", header.offset.multi, header.offset.multi + change_size_by_8);
+ header.offset.multi += change_size_by_8;
+ }
}
/* Adjust length of the section */
From fe70e7d81334ba37614ca5cd0580b2a91a969fc1 Mon Sep 17 00:00:00 2001
From: "Andrew.Liao" <andrew.liao@quantatw.com>
Date: Mon, 26 Sep 2022 17:16:52 +0800
Subject: [PATCH 2/2] fru: Adjust the fru section by their offset order
Originally, ipmitool will assume the FRU section offset will follow a specific order, but this is not true (or not be defined in IPMI FRU SPEC). So change the FRU edit method, now it will:
- Calculate the section offset one by one according to their offset
- Ignore the FRU section offset if its offset is 00 (area does not exist)
- If the new FRU become smaller, reset the redundant data to 0
Fixes #364
---
lib/ipmi_fru.c | 151 +++++++++++++++++++++++++++++--------------------
1 file changed, 90 insertions(+), 61 deletions(-)
diff --git lib/ipmi_fru.c lib/ipmi_fru.c
index a994f3cf..3bf8416d 100644
--- lib/ipmi_fru.c
+++ lib/ipmi_fru.c
@@ -4889,7 +4889,7 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
struct fru_info fru, struct fru_header header,
uint8_t f_type, uint8_t f_index, char *f_string)
{
- int i = 0;
+ int i = 0, j;
uint8_t *fru_data_old = NULL;
uint8_t *fru_data_new = NULL;
uint8_t *fru_area = NULL;
@@ -4901,6 +4901,7 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
uint32_t counter;
unsigned char cksum;
int rc = 1;
+ char section_list[] = {'i', 'c', 'b', 'p', 'm'};
fru_data_old = calloc( fru.size, sizeof(uint8_t) );
@@ -5018,8 +5019,10 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
5) Check if section must be resize. This occur when padding length is not between 0 and 7 */
if( (padding_len < 0) || (padding_len >= 8))
{
- uint32_t remaining_offset = ((header.offset.product * 8) + product_len);
- int change_size_by_8;
+ int change_size_by_8, section_len;
+ char *name;
+ uint8_t *section_offset_by_8;
+ uint8_t last_offset_by_8 = 0;
if(padding_len >= 8)
{
@@ -5044,48 +5047,85 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
#endif
/* Must move sections */
- /* Section that can be modified are as follow
- Chassis
- Board
- product */
+ /* The IPMI FRU SPEC doesn't define the sequence of each FRU area.
+ * Therefore we need to find out the affected section in this change based on
+ * their current offset and adjust each of them.
+ */
- /* Chassis type field */
- if (f_type == 'c' )
- {
- if (header.offset.board != 0) {
- printf("Moving Section Chassis, from %i to %i\n",
- ((header.offset.board) * 8),
- ((header.offset.board + change_size_by_8) * 8)
- );
- memcpy(
- (fru_data_new + ((header.offset.board + change_size_by_8) * 8)),
- (fru_data_old + (header.offset.board) * 8),
- board_len
- );
- header.offset.board += change_size_by_8;
+ /* Find out the section behind the edited section and adjust them */
+ for (j = 0; j < sizeof(section_list); j++) {
+ section_offset_by_8 = NULL;
+ name = NULL;
+
+ switch (section_list[j]) {
+ case 'i':
+ section_offset_by_8 = &header.offset.internal;
+ name = "Internal";
+ break;
+ case 'c':
+ section_offset_by_8 = &header.offset.chassis;
+ name = "Chassis";
+ break;
+ case 'b':
+ section_offset_by_8 = &header.offset.board;
+ name = "Board";
+ break;
+ case 'p':
+ section_offset_by_8 = &header.offset.product;
+ name = "Product";
+ break;
+ case 'm':
+ section_offset_by_8 = &header.offset.multi;
+ name = "MitiRecord";
+ break;
+ default:
+ /* Should not go into here */
+ break;
}
- }
- /* Board type field */
- if ((f_type == 'c' ) || (f_type == 'b' ))
- {
- if (header.offset.product != 0) {
- printf("Moving Section Product, from %i to %i\n",
- ((header.offset.product) * 8),
- ((header.offset.product + change_size_by_8) * 8)
+
+ /* Should never happened */
+ if (section_offset_by_8 == NULL || name == NULL) {
+ continue;
+ }
+
+ /* Ignore the section that doesn't exist */
+ if (*section_offset_by_8 == 0) {
+ continue;
+ }
+
+ /* Store the last offset in case we need to reset the last part */
+ if (last_offset_by_8 < *section_offset_by_8) {
+ last_offset_by_8 = *section_offset_by_8;
+ }
+
+ /* Adjust the section offset that locates behind the current edit section */
+ if (*section_offset_by_8 > (header_offset / 8)) {
+
+ /* Make sure the adjusted offset range is still inside the FRU field */
+ section_len = *(fru_data_old + (*section_offset_by_8 * 8) + 1) * 8;
+ if (((*section_offset_by_8 * 8) + section_len + (change_size_by_8 * 8)) > fru.size)
+ {
+ /* Return error if oversize */
+ printf("Internal error, section %s out of FRU field. %i > %i\n",
+ name,
+ ((*section_offset_by_8 * 8) + section_len + (change_size_by_8 * 8)),
+ fru.size);
+ rc = -1;
+ goto ipmi_fru_set_field_string_rebuild_out;
+ }
+
+ /* Copy the section to adjusted offset */
+ printf("Moving Section %s, from %i to %i\n",
+ name,
+ ((*section_offset_by_8) * 8),
+ ((*section_offset_by_8 + change_size_by_8) * 8)
);
memcpy(
- (fru_data_new + ((header.offset.product + change_size_by_8) * 8)),
- (fru_data_old + (header.offset.product) * 8),
- product_len
+ (fru_data_new + ((*section_offset_by_8 + change_size_by_8) * 8)),
+ (fru_data_old + (*section_offset_by_8) * 8),
+ section_len
);
- header.offset.product += change_size_by_8;
- }
- }
-
- if (header.offset.multi != 0) {
- if ((f_type == 'c' ) || (f_type == 'b' ) || (f_type == 'p' )) {
- printf("Change multi offset from %d to %d\n", header.offset.multi, header.offset.multi + change_size_by_8);
- header.offset.multi += change_size_by_8;
+ *section_offset_by_8 += change_size_by_8;
}
}
@@ -5101,7 +5141,6 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
else if( f_type == 'p')
{
*(fru_data_new + product_offset + 1) += change_size_by_8;
- product_len_new = *(fru_data_new + product_offset + 1) * 8;
}
/* Rebuild Header checksum */
@@ -5116,26 +5155,16 @@ ipmi_fru_set_field_string_rebuild(struct ipmi_intf * intf, uint8_t fruId,
memcpy(fru_data_new, pfru_header, sizeof(struct fru_header));
}
- /* Move remaining sections in 1 copy */
- printf("Moving Remaining Bytes (Multi-Rec , etc..), from %i to %i\n",
- remaining_offset,
- ((header.offset.product) * 8) + product_len_new
- );
- if(((header.offset.product * 8) + product_len_new - remaining_offset) < 0)
- {
- memcpy(
- fru_data_new + (header.offset.product * 8) + product_len_new,
- fru_data_old + remaining_offset,
- fru.size - remaining_offset
- );
- }
- else
- {
- memcpy(
- fru_data_new + (header.offset.product * 8) + product_len_new,
- fru_data_old + remaining_offset,
- fru.size - ((header.offset.product * 8) + product_len_new)
- );
+ /* Reset the last part to 0 if the new FRU is smaller them old one */
+ if (change_size_by_8 < 0) {
+ section_len = *(fru_data_old + (last_offset_by_8 * 8) + 1) * 8;
+
+ /* Get the reset start offset and reset size */
+ int reset_start = ((last_offset_by_8 * 8) + section_len + (change_size_by_8 * 8));
+ int reset_size = (change_size_by_8 * (-1)) * 8;
+
+ printf("Reset to 0 from %i to %i\n", reset_start, reset_start + reset_size);
+ memset(fru_data_new + reset_start, 0, reset_size);
}
}
+33
View File
@@ -0,0 +1,33 @@
From b293d87cac0183ca6872c450cb87d2dc873359a3 Mon Sep 17 00:00:00 2001
From: Daniel Van Allen <dvanallen@google.com>
Date: Wed, 21 Dec 2022 14:38:47 -0500
Subject: [PATCH] lanp: Fix error response from Unsupported Parameter lookup
Return a pointer to the lan_param instead of NULL in the case when the
parameter is not supported.
Resolves ipmitool/ipmitool#388
Signed-off-by Daniel Van Allen <dvanallen@google.com>
---
lib/ipmi_lanp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git lib/ipmi_lanp.c lib/ipmi_lanp.c
index 16c0d9a9..49dc697f 100644
--- lib/ipmi_lanp.c
+++ lib/ipmi_lanp.c
@@ -236,8 +236,12 @@ get_lan_param_select(struct ipmi_intf *intf, uint8_t chan, int param, int select
/* We treat them as valid but empty response */
p->data = NULL;
p->data_len = 0;
- rc = p;
- /* fall through */
+ lprintf(LOG_INFO, "Get LAN Parameter '%s' command unsupported: %s",
+ p->desc,
+ specific_val2str(rsp->ccode,
+ get_lan_cc_vals,
+ completion_code_vals));
+ return p;
default:
/* other completion codes are treated as error */
lprintf(LOG_INFO, "Get LAN Parameter '%s' command failed: %s",
+36
View File
@@ -0,0 +1,36 @@
From 137aeb64cbb493d61d6945cac156aba5f0510780 Mon Sep 17 00:00:00 2001
From: Miao Wang <shankerwangmiao@gmail.com>
Date: Sat, 10 Feb 2024 12:51:15 +0800
Subject: [PATCH] lan: fix lan print fails on unsupported parameters
After upgrading to ipmitool 1.8.19, ipmitool lan print can only print out
`Set in Progress` and other parameters are missing on our servers. After
bisecting, commit:
351dad24a26f lan: Add processing of get/set specific CCs
is identified to be the source of the problem, where the function
get_lan_param_select is expected to consider severial error codes it
receives as empty response. It then constructs an empty response in `p`
and assigns `p` to `rc` and the control flow falls through to the
default case, which prints the error code in verbose mode and should
return `rc` instead of `NULL`.
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
lib/ipmi_lanp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git lib/ipmi_lanp.c lib/ipmi_lanp.c
index fe0046f..26e7365 100644
--- lib/ipmi_lanp.c
+++ lib/ipmi_lanp.c
@@ -245,7 +245,7 @@ get_lan_param_select(struct ipmi_intf *intf, uint8_t chan, int param, int select
specific_val2str(rsp->ccode,
get_lan_cc_vals,
completion_code_vals));
- return NULL;
+ return rc;
}
p->data = rsp->data + 1;
@@ -0,0 +1,35 @@
Description: Fix soensor reading
Author: mareedu srinivasa rao
Origin: upstream, https://sourceforge.net/p/ipmitool/bugs/490/
Bug: https://sourceforge.net/p/ipmitool/bugs/490/
Bug-debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983082
Forwarded: not-needed
Last-Update: 2022-10-29
---
This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
Index: lib/ipmi_sdr.c
===================================================================
--- lib/ipmi_sdr.c
+++ lib/ipmi_sdr.c
@@ -1799,7 +1799,7 @@ ipmi_sdr_print_sensor_fc(struct ipmi_int
sr->s_a_units);
} else /* Discrete */
snprintf(sval, sizeof(sval),
- "0x%02x", sr->s_reading);
+ "0x%02x", sr->s_data2);
}
else if (sr->s_scanning_disabled)
snprintf(sval, sizeof (sval), sr->full ? "disabled" : "Not Readable");
Index: lib/ipmi_sensor.c
===================================================================
--- lib/ipmi_sensor.c
+++ lib/ipmi_sensor.c
@@ -201,7 +201,7 @@ ipmi_sensor_print_fc_discrete(struct ipm
sr->s_a_str, sr->s_a_units, "ok");
} else {
printf("| 0x%-8x | %-10s | 0x%02x%02x",
- sr->s_reading, "discrete",
+ sr->s_data2, "discrete",
sr->s_data2, sr->s_data3);
}
} else {
+75
View File
@@ -0,0 +1,75 @@
From 202f7427e0a4d1f319fc4b914676cc2f08da6c6c Mon Sep 17 00:00:00 2001
From: Alexander Amelkin <alexander@amelkin.msk.ru>
Date: Tue, 17 Sep 2024 15:15:45 +0300
Subject: [PATCH] sdr: Refix 6e037d6bfbbb93b349c8ca331ebde03a (#41)
A bug was introduced by commit 6e037d6bfbbb93b349c8ca331ebde03a837f76bf
due to which the command `ipmitool sdr type` stopped accepting raw
hex values for the type and would only accept strings.
Fix that by partially reverting the troublesome commit.
Additionally, apply the logic of that commit to calls of
`strcasecmp()` in ipmi_sdr.c.
Resolves https://codeberg.org/IPMITool/ipmitool/issues/41
Signed-off-by: Alexander Amelkin <alexander@amelkin.msk.ru>
---
lib/ipmi_sdr.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git lib/ipmi_sdr.c lib/ipmi_sdr.c
index abd4ee1..4732762 100644
--- lib/ipmi_sdr.c
+++ lib/ipmi_sdr.c
@@ -4570,8 +4570,9 @@ ipmi_sdr_print_type(struct ipmi_intf *intf, char *type)
uint8_t sensor_type = 0;
if (!type ||
- strcasecmp(type, "help") == 0 ||
- strcasecmp(type, "list") == 0) {
+ !strcasecmp(type, "help") ||
+ !strcasecmp(type, "list"))
+ {
printf("Sensor Types:\n");
for (x = 1; x < SENSOR_TYPE_MAX; x += 2) {
printf("\t%-25s (0x%02x) %-25s (0x%02x)\n",
@@ -4581,7 +4582,7 @@ ipmi_sdr_print_type(struct ipmi_intf *intf, char *type)
return 0;
}
- if (!strcmp(type, "0x")) {
+ if (!strncmp(type, "0x", 2)) {
/* begins with 0x so let it be entered as raw hex value */
if (str2uchar(type, &sensor_type) != 0) {
lprintf(LOG_ERR,
@@ -4591,7 +4592,7 @@ ipmi_sdr_print_type(struct ipmi_intf *intf, char *type)
}
} else {
for (x = 1; x < SENSOR_TYPE_MAX; x++) {
- if (strcasecmp(sensor_type_desc[x], type) == 0) {
+ if (!strcasecmp(sensor_type_desc[x], type)) {
sensor_type = x;
break;
}
@@ -4638,8 +4639,8 @@ ipmi_sdr_print_entity(struct ipmi_intf *intf, char *entitystr)
int rc = 0;
if (!entitystr ||
- strcasecmp(entitystr, "help") == 0 ||
- strcasecmp(entitystr, "list") == 0) {
+ !strcasecmp(entitystr, "help") ||
+ !strcasecmp(entitystr, "list")) {
print_valstr_2col(entity_id_vals, "Entity IDs", -1);
return 0;
}
@@ -4654,7 +4655,7 @@ ipmi_sdr_print_entity(struct ipmi_intf *intf, char *entitystr)
/* now try string input */
for (i = 0; entity_id_vals[i].str; i++) {
- if (strcasecmp(entitystr, entity_id_vals[i].str) == 0) {
+ if (!strcasecmp(entitystr, entity_id_vals[i].str)) {
entity.id = entity_id_vals[i].val;
entity.instance = 0x7f;
j=1;