pdr: Fix for Entity Association PDR looping
While normalizing the Entity association PDR, record handle
is passed to the library function. Considering PDR contains
logical and physical contained entities, two pdrs with same
record handle are created.
To fix the issue, we are propagating the recently used record
handle from the pldm_pdr_add to entity_association_pdr_add.
Then incrementing the record handle before creating next PDR.
gitlint-ignore: B1, UC1
Fixes: 25ddbccfae0e ("pdr: Add pldm_entity_association_pdr_add_from_node_with_record_handle()")
Change-Id: Ifaa5694cabe7ad38d8881f0b7be0a79d9d3e06c0
Signed-off-by: Archana Kakani <archana.kakani@ibm.com>
diff --git a/src/dsp/pdr.c b/src/dsp/pdr.c
index 142c4f7..95526ff 100644
--- a/src/dsp/pdr.c
+++ b/src/dsp/pdr.c
@@ -795,14 +795,14 @@
return false;
}
-static int entity_association_pdr_add_children(
+static int64_t entity_association_pdr_add_children(
pldm_entity_node *curr, pldm_pdr *repo, uint16_t size,
uint8_t contained_count, uint8_t association_type, bool is_remote,
uint16_t terminus_handle, uint32_t record_handle)
{
uint8_t *start;
uint8_t *pdr;
- int rc;
+ int64_t rc;
pdr = calloc(1, size);
if (!pdr) {
@@ -851,19 +851,26 @@
rc = pldm_pdr_add(repo, pdr, size, is_remote, terminus_handle,
&record_handle);
free(pdr);
- return rc;
+ return (rc < 0) ? rc : record_handle;
}
-static int entity_association_pdr_add_entry(pldm_entity_node *curr,
- pldm_pdr *repo, bool is_remote,
- uint16_t terminus_handle,
- uint32_t record_handle)
+static int64_t entity_association_pdr_add_entry(pldm_entity_node *curr,
+ pldm_pdr *repo, bool is_remote,
+ uint16_t terminus_handle,
+ uint32_t record_handle)
{
uint8_t num_logical_children = pldm_entity_get_num_children(
curr, PLDM_ENTITY_ASSOCIAION_LOGICAL);
uint8_t num_physical_children = pldm_entity_get_num_children(
curr, PLDM_ENTITY_ASSOCIAION_PHYSICAL);
- int rc;
+ int64_t rc;
+
+ if (!num_logical_children && !num_physical_children) {
+ if (record_handle == 0) {
+ return -EINVAL;
+ }
+ return record_handle - 1;
+ }
if (num_logical_children) {
uint16_t logical_pdr_size =
@@ -878,6 +885,12 @@
if (rc < 0) {
return rc;
}
+ if (num_physical_children) {
+ if (rc >= UINT32_MAX) {
+ return -EOVERFLOW;
+ }
+ record_handle = rc + 1;
+ }
}
if (num_physical_children) {
@@ -893,9 +906,10 @@
if (rc < 0) {
return rc;
}
+ record_handle = rc;
}
- return 0;
+ return record_handle;
}
static bool is_present(pldm_entity entity, pldm_entity **entities,
@@ -914,36 +928,56 @@
return false;
}
-static int entity_association_pdr_add(pldm_entity_node *curr, pldm_pdr *repo,
- pldm_entity **entities,
- size_t num_entities, bool is_remote,
- uint16_t terminus_handle,
- uint32_t record_handle)
+static int64_t entity_association_pdr_add(pldm_entity_node *curr,
+ pldm_pdr *repo,
+ pldm_entity **entities,
+ size_t num_entities, bool is_remote,
+ uint16_t terminus_handle,
+ uint32_t record_handle)
{
- int rc;
+ int64_t rc;
if (curr == NULL) {
- return 0;
+ // entity_association_pdr_add function gets called
+ // recursively for the siblings and children of the
+ // entity. This causes NULL current entity node, and the
+ // record handle is returned
+ return record_handle;
}
if (is_present(curr->entity, entities, num_entities)) {
rc = entity_association_pdr_add_entry(
curr, repo, is_remote, terminus_handle, record_handle);
- if (rc) {
+ if (rc < 0) {
return rc;
}
+ if (rc >= UINT32_MAX) {
+ return -EOVERFLOW;
+ }
+ record_handle = rc + 1;
}
rc = entity_association_pdr_add(curr->next_sibling, repo, entities,
num_entities, is_remote,
terminus_handle, record_handle);
- if (rc) {
+ if (rc < 0) {
return rc;
}
+ // entity_association_pdr_add return record handle in success
+ // case. If the pdr gets added to the repo, new record handle
+ // will be returned. Below check confirms if the pdr is added
+ // to the repo and increments the record handle
+ if (record_handle != rc) {
+ if (rc >= UINT32_MAX) {
+ return -EOVERFLOW;
+ }
+ record_handle = rc + 1;
+ }
- return entity_association_pdr_add(curr->first_child, repo, entities,
- num_entities, is_remote,
- terminus_handle, record_handle);
+ rc = entity_association_pdr_add(curr->first_child, repo, entities,
+ num_entities, is_remote,
+ terminus_handle, record_handle);
+ return rc;
}
LIBPLDM_ABI_STABLE
@@ -954,9 +988,10 @@
if (!tree || !repo) {
return 0;
}
-
- return entity_association_pdr_add(tree->root, repo, NULL, 0, is_remote,
- terminus_handle, 0);
+ int64_t rc = entity_association_pdr_add(tree->root, repo, NULL, 0,
+ is_remote, terminus_handle, 0);
+ assert(rc >= INT_MIN);
+ return (rc < 0) ? (int)rc : 0;
}
LIBPLDM_ABI_STABLE
@@ -979,9 +1014,12 @@
return -EINVAL;
}
- return entity_association_pdr_add(node, repo, entities, num_entities,
- is_remote, terminus_handle,
- record_handle);
+ int64_t rc = entity_association_pdr_add(node, repo, entities,
+ num_entities, is_remote,
+ terminus_handle, record_handle);
+
+ assert(rc >= INT_MIN);
+ return (rc < 0) ? (int)rc : 0;
}
static void find_entity_ref_in_tree(pldm_entity_node *tree_node,
@@ -1343,22 +1381,27 @@
sizeof(struct pldm_pdr_entity_association)) {
return;
}
+
struct pldm_pdr_entity_association *entity_association_pdr =
(struct pldm_pdr_entity_association *)start;
- if (entity_association_pdr->num_children == UINT8_MAX) {
+ size_t l_num_entities = entity_association_pdr->num_children;
+
+ if (l_num_entities == 0) {
return;
}
- size_t l_num_entities = entity_association_pdr->num_children + 1;
- if (l_num_entities < 2) {
+ if ((pdr_len - sizeof(struct pldm_pdr_hdr)) / sizeof(pldm_entity) <
+ l_num_entities) {
return;
}
- if (UINT8_MAX < l_num_entities) {
+ if (l_num_entities >= (size_t)UINT8_MAX) {
return;
}
+ l_num_entities++;
+
pldm_entity *l_entities = calloc(l_num_entities, sizeof(pldm_entity));
if (!l_entities) {
return;