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_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());