pdr: Hoist record handle overflow test to avoid memory leak
The rework in 572a39507d1e ("pdr: Introduce pldm_pdr_add_check()") left
pldm_pdr_add_check() vulnerable to a memory leak. If the record handle
counter overflowed then the early-exit reporting the fault left the
memory pointed to by `record` allocated.
Hoist the overflow check over the dynamic allocation so that we don't
have to clean up in the error path.
Fixes: 572a39507d1e ("pdr: Introduce pldm_pdr_add_check()")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I72ba43d009a4e76aa585590909edf6c70d64b7af
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d52a424..431bb63 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@
### Fixed
1. requester: Fix response buffer cast in pldm_send_recv()
+2. pdr: Hoist record handle overflow test to avoid memory leak
## [0.4.0] - 2023-07-14
diff --git a/src/pdr.c b/src/pdr.c
index 639ddd3..31e3be6 100644
--- a/src/pdr.c
+++ b/src/pdr.c
@@ -54,10 +54,24 @@
bool is_remote, uint16_t terminus_handle,
uint32_t *record_handle)
{
+ uint32_t curr;
+
if (!repo || !data || !size || !record_handle) {
return -EINVAL;
}
+ if (*record_handle) {
+ curr = *record_handle;
+ } else if (repo->last) {
+ curr = repo->last->record_handle;
+ if (curr == UINT32_MAX) {
+ return -EOVERFLOW;
+ }
+ curr += 1;
+ } else {
+ curr = 1;
+ }
+
pldm_pdr_record *record = malloc(sizeof(pldm_pdr_record));
if (!record) {
return -ENOMEM;
@@ -75,26 +89,17 @@
record->size = size;
record->is_remote = is_remote;
record->terminus_handle = terminus_handle;
+ record->record_handle = curr;
- if (*record_handle) {
- record->record_handle = *record_handle;
- } else {
- uint32_t curr = repo->last ? repo->last->record_handle : 0;
- if (curr == UINT32_MAX) {
- return -EOVERFLOW;
- }
- record->record_handle = curr + 1;
-
- if (data != NULL) {
- /* If record handle is 0, that is an indication for this API to
- * compute a new handle. For that reason, the computed handle
- * needs to be populated in the PDR header. For a case where the
- * caller supplied the record handle, it would exist in the
- * header already.
- */
- struct pldm_pdr_hdr *hdr = (void *)record->data;
- hdr->record_handle = htole32(record->record_handle);
- }
+ if (!*record_handle && data) {
+ /* If record handle is 0, that is an indication for this API to
+ * compute a new handle. For that reason, the computed handle
+ * needs to be populated in the PDR header. For a case where the
+ * caller supplied the record handle, it would exist in the
+ * header already.
+ */
+ struct pldm_pdr_hdr *hdr = (void *)record->data;
+ hdr->record_handle = htole32(record->record_handle);
}
record->next = NULL;