From 81011685ea5e8897f8c0971eca5feb93c6880f09 Mon Sep 17 00:00:00 2001 From: Andrew Liao 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" 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); } }