bios_table: pldm_bios_table_iter_next(): Invalid entry halts iteration

The attribute iterator machinary prevented misbehaviour through use of
assert(). The attribute list is maintained as a linear sequence of
variably sized data structures that are packed against each other in the
address space. The iterator is implemented by assigning a callback that
can determine the length of each entry as appropriate for the entry's
type, and then moving the iterator's cursor between elements.

The length derivation for some elements was protected by assert(). To
avoid the asserts we rework the length callback prototype to return
a signed size value and indicate an error state with a negative size.

pldm_bios_table_iter_next() is reworked to detect the error case on
deriving the element size (negative size) and behave as if the iterator
has terminated.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I80db3fe179201b169acc68c68633d8dd3f3a6334
diff --git a/src/bios_table.c b/src/bios_table.c
index 910a499..ec96b82 100644
--- a/src/bios_table.c
+++ b/src/bios_table.c
@@ -4,6 +4,7 @@
 #include "utils.h"
 #include <assert.h>
 #include <endian.h>
+#include <limits.h>
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -111,11 +112,15 @@
 	return PLDM_SUCCESS;
 }
 
-static size_t string_table_entry_length(const void *table_entry)
+static ssize_t string_table_entry_length(const void *table_entry)
 {
 	const struct pldm_bios_string_table_entry *entry = table_entry;
-	return sizeof(*entry) - sizeof(entry->name) +
-	       pldm_bios_table_string_entry_decode_string_length(entry);
+	size_t len = sizeof(*entry) - sizeof(entry->name) +
+		     pldm_bios_table_string_entry_decode_string_length(entry);
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 static int get_bios_attr_handle(uint16_t *val)
@@ -291,12 +296,17 @@
 
 /** @brief Get length of an enum attribute entry
  */
-static size_t attr_table_entry_length_enum(const void *arg)
+static ssize_t attr_table_entry_length_enum(const void *arg)
 {
 	const struct pldm_bios_attr_table_entry *entry = arg;
 	uint8_t pv_num = entry->metadata[0];
 	uint8_t def_num = pldm_bios_table_attr_entry_enum_decode_def_num(entry);
-	return pldm_bios_table_attr_entry_enum_encode_length(pv_num, def_num);
+	size_t len =
+		pldm_bios_table_attr_entry_enum_encode_length(pv_num, def_num);
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 struct attr_table_string_entry_fields {
@@ -456,12 +466,17 @@
 
 /** @brief Get length of a string attribute entry
  */
-static size_t attr_table_entry_length_string(const void *entry)
+static ssize_t attr_table_entry_length_string(const void *entry)
 {
 	uint16_t def_str_len =
 		pldm_bios_table_attr_entry_string_decode_def_string_length(
 			entry);
-	return pldm_bios_table_attr_entry_string_encode_length(def_str_len);
+	size_t len =
+		pldm_bios_table_attr_entry_string_encode_length(def_str_len);
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 struct attr_table_integer_entry_fields {
@@ -572,15 +587,19 @@
 	*def = le64toh(fields->default_value);
 }
 
-static size_t attr_table_entry_length_integer(const void *entry)
+static ssize_t attr_table_entry_length_integer(const void *entry)
 {
 	(void)entry;
-	return pldm_bios_table_attr_entry_integer_encode_length();
+	size_t len = pldm_bios_table_attr_entry_integer_encode_length();
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 struct table_entry_length {
 	uint8_t attr_type;
-	size_t (*entry_length_handler)(const void *);
+	ssize_t (*entry_length_handler)(const void *);
 };
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
@@ -614,7 +633,7 @@
 	  .entry_length_handler = attr_table_entry_length_integer },
 };
 
-static size_t attr_table_entry_length(const void *table_entry)
+static ssize_t attr_table_entry_length(const void *table_entry)
 {
 	const struct pldm_bios_attr_table_entry *entry = table_entry;
 	const struct table_entry_length *attr_table_entry =
@@ -622,7 +641,13 @@
 						attr_table_entries,
 						ARRAY_SIZE(attr_table_entries));
 	assert(attr_table_entry != NULL);
+	if (!attr_table_entry) {
+		return -1;
+	}
 	assert(attr_table_entry->entry_length_handler != NULL);
+	if (!attr_table_entry->entry_length_handler) {
+		return -1;
+	}
 
 	return attr_table_entry->entry_length_handler(entry);
 }
@@ -703,11 +728,16 @@
 	return PLDM_SUCCESS;
 }
 
-static size_t attr_value_table_entry_length_enum(const void *entry)
+static ssize_t attr_value_table_entry_length_enum(const void *entry)
 {
 	uint8_t number =
 		pldm_bios_table_attr_value_entry_enum_decode_number(entry);
-	return pldm_bios_table_attr_value_entry_encode_enum_length(number);
+	size_t len =
+		pldm_bios_table_attr_value_entry_encode_enum_length(number);
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 LIBPLDM_ABI_STABLE
@@ -774,12 +804,16 @@
 	return PLDM_SUCCESS;
 }
 
-static size_t attr_value_table_entry_length_string(const void *entry)
+static ssize_t attr_value_table_entry_length_string(const void *entry)
 {
 	uint16_t str_length =
 		pldm_bios_table_attr_value_entry_string_decode_length(entry);
-	return pldm_bios_table_attr_value_entry_encode_string_length(
+	size_t len = pldm_bios_table_attr_value_entry_encode_string_length(
 		str_length);
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 LIBPLDM_ABI_STABLE
@@ -832,10 +866,14 @@
 	return cv;
 }
 
-static size_t attr_value_table_entry_length_integer(const void *entry)
+static ssize_t attr_value_table_entry_length_integer(const void *entry)
 {
 	(void)entry;
-	return pldm_bios_table_attr_value_entry_encode_integer_length();
+	size_t len = pldm_bios_table_attr_value_entry_encode_integer_length();
+	if (len > SSIZE_MAX) {
+		return -1;
+	}
+	return (ssize_t)len;
 }
 
 static const struct table_entry_length attr_value_table_entries[] = {
@@ -853,7 +891,7 @@
 	  .entry_length_handler = attr_value_table_entry_length_integer },
 };
 
-static size_t attr_value_table_entry_length(const void *table_entry)
+static ssize_t attr_value_table_entry_length(const void *table_entry)
 {
 	const struct pldm_bios_attr_val_table_entry *entry = table_entry;
 	const struct table_entry_length *entry_length =
@@ -861,7 +899,13 @@
 			entry->attr_type, attr_value_table_entries,
 			ARRAY_SIZE(attr_value_table_entries));
 	assert(entry_length != NULL);
+	if (!entry_length) {
+		return -1;
+	}
 	assert(entry_length->entry_length_handler != NULL);
+	if (!entry_length->entry_length_handler) {
+		return -1;
+	}
 
 	return entry_length->entry_length_handler(entry);
 }
@@ -951,7 +995,7 @@
 	const uint8_t *table_data;
 	size_t table_len;
 	size_t current_pos;
-	size_t (*entry_length_handler)(const void *table_entry);
+	ssize_t (*entry_length_handler)(const void *table_entry);
 };
 
 LIBPLDM_ABI_STABLE
@@ -993,10 +1037,15 @@
 LIBPLDM_ABI_STABLE
 bool pldm_bios_table_iter_is_end(const struct pldm_bios_table_iter *iter)
 {
+	ssize_t len;
+
 	if (iter->table_len - iter->current_pos <= pad_and_check_max) {
 		return true;
 	}
-	return false;
+
+	len = iter->entry_length_handler(iter->table_data + iter->current_pos);
+
+	return len < 0;
 }
 
 LIBPLDM_ABI_STABLE
@@ -1006,7 +1055,12 @@
 		return;
 	}
 	const void *entry = iter->table_data + iter->current_pos;
-	iter->current_pos += iter->entry_length_handler(entry);
+	ssize_t rc = iter->entry_length_handler(entry);
+	/* Prevent bad behaviour by acting as if we've hit the end of the iterator */
+	if (rc < 0) {
+		return;
+	}
+	iter->current_pos += rc;
 }
 
 LIBPLDM_ABI_STABLE