pldm: Convert to pldm_bios_table_*_check() APIs
The pldm_bios_table_*_check() APIs wrap around unchecked equivalents
that only sanitize their parameters using assert(). The checks included
in the pldm_bios_table_*_check() APIs test the same conditions that
would trigger the assert()s in the unchecked APIs but instead return an
error code on failure.
Use of the unchecked APIs has the potential to blow up the pldmd
process, but also requires that libpldm continue to be built with
`-UNDEBUG` to avoid undefined behaviour (by aborting the process). This
impacts the performance of the library in addition to be a bit of a UX
disaster.
The unchecked APIs are deprecated in libpldm as a step improve its
safety and code generation:
https://gerrit.openbmc.org/c/openbmc/libpldm/+/64179
Tested: Booted the host on a p10bmc system with the patch applied.
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I9677a7fa5ca59d04865963b367f3bb55c8676cfb
diff --git a/libpldmresponder/bios_config.cpp b/libpldmresponder/bios_config.cpp
index 1e74081..a9d7723 100644
--- a/libpldmresponder/bios_config.cpp
+++ b/libpldmresponder/bios_config.cpp
@@ -169,13 +169,17 @@
case PLDM_BIOS_ENUMERATION:
case PLDM_BIOS_ENUMERATION_READ_ONLY:
{
- auto pvNum =
- pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
+ uint8_t pvNum;
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_num_check(entry,
+ &pvNum);
std::vector<uint16_t> pvHandls(pvNum);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
entry, pvHandls.data(), pvHandls.size());
- auto defNum =
- pldm_bios_table_attr_entry_enum_decode_def_num(entry);
+ uint8_t defNum;
+ pldm_bios_table_attr_entry_enum_decode_def_num_check(entry,
+ &defNum);
std::vector<uint8_t> defIndices(defNum);
pldm_bios_table_attr_entry_enum_decode_def_indices(
entry, defIndices.data(), defIndices.size());
@@ -264,8 +268,10 @@
auto strLength =
pldm_bios_table_string_entry_decode_string_length(stringEntry);
std::vector<char> buffer(strLength + 1 /* sizeof '\0' */);
- pldm_bios_table_string_entry_decode_string(stringEntry, buffer.data(),
- buffer.size());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_string_entry_decode_string_check(
+ stringEntry, buffer.data(), buffer.size());
+
attributeName = std::string(buffer.data(), buffer.data() + strLength);
if (!biosAttributes.empty())
@@ -292,7 +298,9 @@
pldm_bios_table_string_entry_decode_string_length(
stringEntry);
std::vector<char> buffer(strLength + 1 /* sizeof '\0' */);
- pldm_bios_table_string_entry_decode_string(
+ // Preconditions are upheld therefore no error check
+ // necessary
+ pldm_bios_table_string_entry_decode_string_check(
stringEntry, buffer.data(), buffer.size());
return std::string(buffer.data(),
@@ -302,10 +310,13 @@
attributeType = "xyz.openbmc_project.BIOSConfig.Manager."
"AttributeType.Enumeration";
- auto pvNum =
- pldm_bios_table_attr_entry_enum_decode_pv_num(attrEntry);
+ uint8_t pvNum;
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_num_check(attrEntry,
+ &pvNum);
std::vector<uint16_t> pvHandls(pvNum);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
attrEntry, pvHandls.data(), pvHandls.size());
// get possible_value
@@ -330,8 +341,10 @@
currentValue = getValue(pvHandls[handles[i]], *stringTable);
}
- auto defNum =
- pldm_bios_table_attr_entry_enum_decode_def_num(attrEntry);
+ uint8_t defNum;
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_def_num_check(attrEntry,
+ &defNum);
std::vector<uint8_t> defIndices(defNum);
pldm_bios_table_attr_entry_enum_decode_def_indices(
attrEntry, defIndices.data(), defIndices.size());
@@ -388,9 +401,10 @@
attrEntry);
auto max = pldm_bios_table_attr_entry_string_decode_max_length(
attrEntry);
- auto def =
- pldm_bios_table_attr_entry_string_decode_def_string_length(
- attrEntry);
+ uint16_t def;
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_string_decode_def_string_length_check(
+ attrEntry, &def);
std::vector<char> defString(def + 1);
pldm_bios_table_attr_entry_string_decode_def_string(
attrEntry, defString.data(), defString.size());
@@ -632,8 +646,9 @@
auto strLength =
pldm_bios_table_string_entry_decode_string_length(stringEntry);
std::vector<char> buffer(strLength + 1 /* sizeof '\0' */);
- pldm_bios_table_string_entry_decode_string(stringEntry, buffer.data(),
- buffer.size());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_string_entry_decode_string_check(stringEntry, buffer.data(),
+ buffer.size());
return std::string(buffer.data(), buffer.data() + strLength);
}
@@ -644,10 +659,22 @@
{
auto attrEntry = pldm_bios_table_attr_find_by_handle(
attrTable->data(), attrTable->size(), handle);
- auto pvNum = pldm_bios_table_attr_entry_enum_decode_pv_num(attrEntry);
+ uint8_t pvNum;
+ int rc = pldm_bios_table_attr_entry_enum_decode_pv_num_check(attrEntry,
+ &pvNum);
+ if (rc != PLDM_SUCCESS)
+ {
+ error(
+ "Failed to decode BIOS table possible values for attribute entry: {LIPBLDM_ERROR}",
+ "LIBPLDM_ERROR", rc);
+ throw std::runtime_error(
+ "Failed to decode BIOS table possible values for attribute entry");
+ }
+
std::vector<uint16_t> pvHandls(pvNum);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(attrEntry, pvHandls.data(),
- pvHandls.size());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
+ attrEntry, pvHandls.data(), pvHandls.size());
std::string displayString = std::to_string(pvHandls[index]);
diff --git a/libpldmresponder/bios_table.cpp b/libpldmresponder/bios_table.cpp
index 5cfd372..898a34a 100644
--- a/libpldmresponder/bios_table.cpp
+++ b/libpldmresponder/bios_table.cpp
@@ -4,6 +4,8 @@
#include <libpldm/bios_table.h>
#include <libpldm/utils.h>
+#include <phosphor-logging/lg2.hpp>
+
#include <fstream>
namespace pldm
@@ -98,8 +100,9 @@
{
auto strLength = pldm_bios_table_string_entry_decode_string_length(entry);
std::vector<char> buffer(strLength + 1 /* sizeof '\0' */);
- pldm_bios_table_string_entry_decode_string(entry, buffer.data(),
- buffer.size());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_string_entry_decode_string_check(entry, buffer.data(),
+ buffer.size());
return std::string(buffer.data(), buffer.data() + strLength);
}
const pldm_bios_string_table_entry* constructEntry(Table& table,
@@ -108,8 +111,9 @@
auto tableSize = table.size();
auto entryLength = pldm_bios_table_string_entry_encode_length(str.length());
table.resize(tableSize + entryLength);
- pldm_bios_table_string_entry_encode(table.data() + tableSize, entryLength,
- str.c_str(), str.length());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_string_entry_encode_check(
+ table.data() + tableSize, entryLength, str.c_str(), str.length());
return reinterpret_cast<pldm_bios_string_table_entry*>(table.data() +
tableSize);
}
@@ -149,8 +153,15 @@
auto tableSize = table.size();
table.resize(tableSize + entryLength, 0);
- pldm_bios_table_attr_entry_string_encode(table.data() + tableSize,
- entryLength, info);
+ int rc = pldm_bios_table_attr_entry_string_encode_check(
+ table.data() + tableSize, entryLength, info);
+ if (rc != PLDM_SUCCESS)
+ {
+ lg2::error("Failed to encode BIOS table string entry: {LIBPLDM_ERROR}",
+ "LIBPLDM_ERROR", rc);
+ throw std::runtime_error("Failed to encode BIOS table string entry");
+ }
+
return reinterpret_cast<pldm_bios_attr_table_entry*>(table.data() +
tableSize);
}
@@ -173,8 +184,17 @@
auto strType = pldm_bios_table_attr_entry_string_decode_string_type(entry);
auto minLength = pldm_bios_table_attr_entry_string_decode_min_length(entry);
auto maxLength = pldm_bios_table_attr_entry_string_decode_max_length(entry);
- auto defLength =
- pldm_bios_table_attr_entry_string_decode_def_string_length(entry);
+ uint16_t defLength;
+ int rc = pldm_bios_table_attr_entry_string_decode_def_string_length_check(
+ entry, &defLength);
+ if (rc != PLDM_SUCCESS)
+ {
+ lg2::error(
+ "Failed to decode BIOS table string definition length: {LIBPLDM_ERROR}",
+ "LIBPLDM_ERROR", rc);
+ throw std::runtime_error(
+ "Failed to decode BIOS table string definitionlength");
+ }
std::vector<char> buffer(defLength + 1);
pldm_bios_table_attr_entry_string_decode_def_string(entry, buffer.data(),
@@ -201,8 +221,9 @@
auto tableSize = table.size();
table.resize(tableSize + entryLength, 0);
- pldm_bios_table_attr_entry_enum_encode(table.data() + tableSize,
- entryLength, info);
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_encode_check(table.data() + tableSize,
+ entryLength, info);
return reinterpret_cast<pldm_bios_attr_table_entry*>(table.data() +
tableSize);
@@ -210,10 +231,23 @@
EnumField decodeEnumEntry(const pldm_bios_attr_table_entry* entry)
{
- uint8_t pvNum = pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
+ uint8_t pvNum;
+ int rc = pldm_bios_table_attr_entry_enum_decode_pv_num_check(entry, &pvNum);
+ if (rc != PLDM_SUCCESS)
+ {
+ lg2::error(
+ "Failed to decode the number of possible values for BIOS table enum entry: {LIBPLDM_ERROR}",
+ "LIBPLDM_ERROR", rc);
+ throw std::runtime_error(
+ "Failed to decode the number of possible values for BIOS table enum entry");
+ }
std::vector<uint16_t> pvHdls(pvNum, 0);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(entry, pvHdls.data(), pvNum);
- auto defNum = pldm_bios_table_attr_entry_enum_decode_def_num(entry);
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(entry, pvHdls.data(),
+ pvNum);
+ // Preconditions are upheld therefore no error check necessary
+ uint8_t defNum;
+ pldm_bios_table_attr_entry_enum_decode_def_num_check(entry, &defNum);
std::vector<uint8_t> defIndices(defNum, 0);
pldm_bios_table_attr_entry_enum_decode_def_indices(entry, defIndices.data(),
defIndices.size());
diff --git a/pldmtool/pldm_bios_cmd.cpp b/pldmtool/pldm_bios_cmd.cpp
index 7c0adc5..6cf3e37 100644
--- a/pldmtool/pldm_bios_cmd.cpp
+++ b/pldmtool/pldm_bios_cmd.cpp
@@ -285,8 +285,9 @@
auto strLength =
pldm_bios_table_string_entry_decode_string_length(stringEntry);
std::vector<char> buffer(strLength + 1 /* sizeof '\0' */);
- pldm_bios_table_string_entry_decode_string(stringEntry, buffer.data(),
- buffer.size());
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_string_entry_decode_string_check(
+ stringEntry, buffer.data(), buffer.size());
return std::string(buffer.data(), buffer.data() + strLength);
}
@@ -332,9 +333,16 @@
{
return displayString;
}
- auto pvNum = pldm_bios_table_attr_entry_enum_decode_pv_num(attrEntry);
+ uint8_t pvNum;
+ int rc = pldm_bios_table_attr_entry_enum_decode_pv_num_check(attrEntry,
+ &pvNum);
+ if (rc != PLDM_SUCCESS)
+ {
+ return displayString;
+ }
std::vector<uint16_t> pvHandls(pvNum);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
attrEntry, pvHandls.data(), pvHandls.size());
return displayStringHandle(pvHandls[index], stringTable, false);
}
@@ -554,13 +562,21 @@
case PLDM_BIOS_ENUMERATION:
case PLDM_BIOS_ENUMERATION_READ_ONLY:
{
- auto pvNum =
- pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
+ uint8_t pvNum;
+ // Preconditions are upheld therefore no error check
+ // necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_num_check(entry,
+ &pvNum);
std::vector<uint16_t> pvHandls(pvNum);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+ // Preconditions are upheld therefore no error check
+ // necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
entry, pvHandls.data(), pvHandls.size());
- auto defNum =
- pldm_bios_table_attr_entry_enum_decode_def_num(entry);
+ uint8_t defNum;
+ // Preconditions are upheld therefore no error check
+ // necessary
+ pldm_bios_table_attr_entry_enum_decode_def_num_check(
+ entry, &defNum);
std::vector<uint8_t> defIndices(defNum);
pldm_bios_table_attr_entry_enum_decode_def_indices(
entry, defIndices.data(), defIndices.size());
@@ -609,9 +625,11 @@
auto max =
pldm_bios_table_attr_entry_string_decode_max_length(
entry);
- auto def =
- pldm_bios_table_attr_entry_string_decode_def_string_length(
- entry);
+ uint16_t def;
+ // Preconditions are upheld therefore no error check
+ // necessary
+ pldm_bios_table_attr_entry_string_decode_def_string_length_check(
+ entry, &def);
std::vector<char> defString(def + 1);
pldm_bios_table_attr_entry_string_decode_def_string(
entry, defString.data(), defString.size());
@@ -807,10 +825,13 @@
{
entryLength =
pldm_bios_table_attr_value_entry_encode_enum_length(1);
- auto pvNum =
- pldm_bios_table_attr_entry_enum_decode_pv_num(attrEntry);
+ uint8_t pvNum;
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_num_check(attrEntry,
+ &pvNum);
std::vector<uint16_t> pvHdls(pvNum, 0);
- pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+ // Preconditions are upheld therefore no error check necessary
+ pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
attrEntry, pvHdls.data(), pvNum);
auto stringEntry = pldm_bios_table_string_find_by_string(
stringTable->data(), stringTable->size(),