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 ha

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

show more ...

requester: Fix response buffer cast in pldm_send_recv()

With the latest libpldm code version 0.4, sometimes the calling
`pldmtool platform GetPDR` command while polling the sensors will be
ended wit

requester: Fix response buffer cast in pldm_send_recv()

With the latest libpldm code version 0.4, sometimes the calling
`pldmtool platform GetPDR` command while polling the sensors will be
ended with the errors `Failed to receive RC = 3`. This error code is
printed when the `pldm_send_recv()` responses
PLDM_REQUESTER_NOT_RESP_MSG.
In the `pldm_send_recv()`, `hdr` is `struct pldm_msg_hdr` pointer and
`pldm_resp_msg` is a double pointer, type casting the double pointer to
the pointer is incorrect.

Tested:
1. Call `pldmtool platform GetPDR` command while running the sensor
reading.
2. No `Failed to receive RC = 3`

Fixes: 0411b712b746 ("transport: Make APIs work for all types of messages")
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Change-Id: I1efadc80bbd8803d0b97cb634eb8bd7df9d279b9

show more ...

fru: Use inequality to judge table bounds check

If `p` points to any address beyond the end of the table then the table
should be considered completely processed. Requiring an exact match is a
recip

fru: Use inequality to judge table bounds check

If `p` points to any address beyond the end of the table then the table
should be considered completely processed. Requiring an exact match is a
recipe for trouble.

Fixes: #10
Reported-by: Frederic Barrat <fbarrat@linux.ibm.com>
Suggested-by: Frederic Barrat <fbarrat@linux.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Ia092d874411c58e0eba4f210001e6cda4d3b8695

show more ...

libpldm: Lift or remove asserts where a subsequent check exists

In the case where an existing public API relies on assert(), move any
asserts from its underlying `*_check()` equivalent up into to th

libpldm: Lift or remove asserts where a subsequent check exists

In the case where an existing public API relies on assert(), move any
asserts from its underlying `*_check()` equivalent up into to the
unchecked function implementation.

Everywhere else, remove asserts where they are unnecessary as the API is
capable of reporting errors.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Iff8ce32b5d5e08ba1244e17d58722a556eca8694

show more ...

pdr: pldm_entity_get_num_children(): Don't return invalid values

Allow for future elision of the assertion.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Iea112ec0e7c8a659a8819031b35a8

pdr: pldm_entity_get_num_children(): Don't return invalid values

Allow for future elision of the assertion.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Iea112ec0e7c8a659a8819031b35a8869d1e21545

show more ...

pdr: pldm_entity_association_pdr_extract(): Return early if necessary

Now that we're no longer mutating out-parameters unless we're
successful, return early if success is not possible.

Signed-off-b

pdr: pldm_entity_association_pdr_extract(): Return early if necessary

Now that we're no longer mutating out-parameters unless we're
successful, return early if success is not possible.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I6ed9b584d836ae879531f136f6125b90b6185920

show more ...

pdr: pldm_entity_association_pdr_extract(): Assign out params at exit

Avoid assigning results to out-parameters before we know we can't fail.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-

pdr: pldm_entity_association_pdr_extract(): Assign out params at exit

Avoid assigning results to out-parameters before we know we can't fail.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I7b99d0a04471e97ae6bf13c2467dd938c57c1485

show more ...

pdr: pldm_entity_association_pdr_extract(): Hoist assert() over malloc()

The asserted condition was unrelated to the invocation of malloc().
Instead, put it adjacent to the assignment of `*num_entit

pdr: pldm_entity_association_pdr_extract(): Hoist assert() over malloc()

The asserted condition was unrelated to the invocation of malloc().
Instead, put it adjacent to the assignment of `*num_entites`, which is
the quantity used to derive the asserted property.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Iadbfe9a68e913fb7e7ea353e4964b9e9bc62fda4

show more ...

pdr: pldm_entity_association_pdr_extract(): Use array notation

Stop with pointer arithmetic and make it clear what the intent of the
operations are.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

pdr: pldm_entity_association_pdr_extract(): Use array notation

Stop with pointer arithmetic and make it clear what the intent of the
operations are.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Ia232fd569dc603bb80998954a16e4a947e9edc4e

show more ...

pdr: pldm_entity_association_pdr_extract(): Use a for-loop

The while-loop initialisation and maintenance can be trivially
compressed by a for-loop.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
C

pdr: pldm_entity_association_pdr_extract(): Use a for-loop

The while-loop initialisation and maintenance can be trivially
compressed by a for-loop.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I591e2c55f8e340d637d69001d6a0a630100921af

show more ...

bios_table: Annotate pldm_bios_table_attr_value_entry_encode_integer()

0088a6ae4cec ("bios_table: Deprecate
pldm_bios_table_attr_value_entry_encode_integer()") did the work to
deprecate the function

bios_table: Annotate pldm_bios_table_attr_value_entry_encode_integer()

0088a6ae4cec ("bios_table: Deprecate
pldm_bios_table_attr_value_entry_encode_integer()") did the work to
deprecate the function but failed to actually mark it as such. Fix that
now.

Fixes: 0088a6ae4cec ("bios_table: Deprecate pldm_bios_table_attr_value_entry_encode_integer()")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I84e067a3b6ad8a39100774b25feac76703153e93

show more ...

pdr: Stabilise pldm_pdr_add_fru_record_set_check()

pldm_pdr_add_fru_record_set_check() is a replacement for
pldm_pdr_add_fru_record_set(). The latter uses assert() to sanitize its
arguments, while t

pdr: Stabilise pldm_pdr_add_fru_record_set_check()

pldm_pdr_add_fru_record_set_check() is a replacement for
pldm_pdr_add_fru_record_set(). The latter uses assert() to sanitize its
arguments, while the former instead returns a value indicating success
or failure.

Use of pldm_pdr_add_fru_record_set_check() is demonstrated in the
following patch:

https://gerrit.openbmc.org/c/openbmc/pldm/+/64631

Additionally, deprecate pldm_pdr_add_fru_record_set() now that there is
a stable replacement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I706b2a8ed6c5468390b1e4a7b14fa60509dd9c37

show more ...

pdr: Stabilise pldm_pdr_add_check()

pldm_pdr_add_check() is a replacement for pldm_pdr_add(). The latter
used assert() to sanitize its arguments while the former instead returns
a value indicating s

pdr: Stabilise pldm_pdr_add_check()

pldm_pdr_add_check() is a replacement for pldm_pdr_add(). The latter
used assert() to sanitize its arguments while the former instead returns
a value indicating success or failure.

Use of pldm_pdr_add_check() is demonstrated in the following patch:

https://gerrit.openbmc.org/c/openbmc/pldm/+/64630

Additionally, deprecate pldm_pdr_add() now that there is a stable
replacement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I3963c6f8bc869df5772e10e66e09e34b91776710

show more ...

pdr: Stabilise pldm_entity_association_pdr_add_from_node_check()

pldm_entity_association_pdr_add_from_node_check() is a replacement for
pldm_entity_association_pdr_add_from_node(). The latter saniti

pdr: Stabilise pldm_entity_association_pdr_add_from_node_check()

pldm_entity_association_pdr_add_from_node_check() is a replacement for
pldm_entity_association_pdr_add_from_node(). The latter sanitized its
arguments with assert(), while the former instead returns a value
indicating success or error.

Use of pldm_entity_association_pdr_add_from_node_check() is demonstrated
here:

https://gerrit.openbmc.org/c/openbmc/pldm/+/64629

Additionally, deprecate pldm_entity_association_pdr_add_from_node() now
that there is a stable replacement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: If328d4e0ad395fffc01ee79b1a24904ee1de7edf

show more ...

fru: Stabilise get_fru_record_by_option_check()

get_fru_record_by_option_check() is a replacement for
get_fru_record_by_option(). The latter sanitized its arguments using
assert() while the former i

fru: Stabilise get_fru_record_by_option_check()

get_fru_record_by_option_check() is a replacement for
get_fru_record_by_option(). The latter sanitized its arguments using
assert() while the former instead returns a value indicating success or
failure.

Use of get_fru_record_by_option_check() is demonstrated in the following
patch:

https://gerrit.openbmc.org/c/openbmc/pldm/+/64628

Additionally, deprecate get_fru_record_by_option() now that there's a
stable replacement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I80c77b12606ffe6aa5c38086fc162bbe34f2dd2f

show more ...

bios_table: Stabilise pldm_bios_table_append_pad_checksum_check()

pldm_bios_table_append_pad_checksum_check() is a replacement for
pldm_bios_table_append_pad_checksum(), which used assert() to sanit

bios_table: Stabilise pldm_bios_table_append_pad_checksum_check()

pldm_bios_table_append_pad_checksum_check() is a replacement for
pldm_bios_table_append_pad_checksum(), which used assert() to sanitize
its arguments. pldm_bios_table_append_pad_checksum_check() instead
returns an error.

Use of pldm_bios_table_append_pad_checksum_check() is demonstrated in
the following patch:

https://gerrit.openbmc.org/c/openbmc/pldm/+/64626

Additionally, deprecate pldm_bios_table_append_pad_checksum() now that
there's a stable replacement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: If9fc91ac60477cfb6ba96e377dbf5768a990bd69

show more ...

transport: Fix possible NULL ptr deref in pldm_socket_sndbuf_init()

From cppcheck:
```
src/transport/socket.c:24:10: warning: Either the condition 'fp==NULL'
is redundant or there is possible null p

transport: Fix possible NULL ptr deref in pldm_socket_sndbuf_init()

From cppcheck:
```
src/transport/socket.c:24:10: warning: Either the condition 'fp==NULL'
is redundant or there is possible null pointer dereference: fp.
[nullPointerRedundantCheck]
fclose(fp);
^
src/transport/socket.c:23:9: note: Assuming that condition 'fp==NULL' is
not redundant
if (fp == NULL || fgets(line, sizeof(line), fp) == NULL) {
^
src/transport/socket.c:24:10: note: Null pointer dereference
fclose(fp);
^
```

Fixes: 04273e9f6895 ("Resize socket send buffer if needed")
Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
Change-Id: Iac053a3d3c784e3592eea027f25f005dba024850

show more ...

transport: Update internal header file

Rename the header guard to be in-line with other internal headers and
less likely to conflict with other headers. Also remove the c++ guard as
we don't need it

transport: Update internal header file

Rename the header guard to be in-line with other internal headers and
less likely to conflict with other headers. Also remove the c++ guard as
we don't need it in an internal header.

Signed-off-by: Rashmica Gupta <rashmica@linux.ibm.com>
Change-Id: I86ef4d478fca62a5532fbc440216e0898a463821

show more ...

pdr: Introduce pldm_pdr_add_fru_record_set_check()

pldm_pdr_add_fru_record_set() relied on assert() to communicate failure
to the caller. Introduce pldm_pdr_add_fru_record_set_check() which
instead

pdr: Introduce pldm_pdr_add_fru_record_set_check()

pldm_pdr_add_fru_record_set() relied on assert() to communicate failure
to the caller. Introduce pldm_pdr_add_fru_record_set_check() which
instead returns a value indicating success or failure.
pldm_pdr_add_fru_record_set() will be deprecated once
pldm_pdr_add_fru_record_set_check() is stabilised.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I1e09a5663895f1e09b5e40afe5db4b4864d3496e

show more ...

pdr: Introduce pldm_pdr_add_check()

pldm_pdr_add() relied on assert() to communicate failure to the caller.
Introduce pldm_pdr_add_check() which instead returns a value indicating
success or failure

pdr: Introduce pldm_pdr_add_check()

pldm_pdr_add() relied on assert() to communicate failure to the caller.
Introduce pldm_pdr_add_check() which instead returns a value indicating
success or failure. pldm_pdr_add() will be deprecated once
pldm_pdr_add_check() is stabilised.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I43542e8e7411edb32a7afa242f1ae68d6fe3ad89

show more ...

pdr: Inline get_new_record_handle() into pldm_pdr_add()

This yields improved readability with fewer lines.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I590a0337bc9ec8f4d2c19f9ac33c60

pdr: Inline get_new_record_handle() into pldm_pdr_add()

This yields improved readability with fewer lines.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I590a0337bc9ec8f4d2c19f9ac33c60d5e97589d7
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

pdr: Further constrain first and last pointer semantics

The property that we're trying to uphold across pldm_pdr's first and
last members is that they are either both NULL, or both point to valid
re

pdr: Further constrain first and last pointer semantics

The property that we're trying to uphold across pldm_pdr's first and
last members is that they are either both NULL, or both point to valid
records that are reachable from each other.

Properly specify this property by hoisting the assert out of the
if-condition body and adjusting it to cover the behaviour of both
branches. Hoisting the assertion resolves the following issue identified
by clang-tidy:

```
../src/pdr.c:83:20: error: Access to field 'next' results in a dereference of a null pointer (loaded from field 'last') [clang-analyzer-core.NullDereference,-warnings-as-errors]
repo->last->next = record;
^
../src/pdr.c:285:9: note: Calling 'pldm_pdr_add'
return pldm_pdr_add(repo, data, size, bmc_record_handle, false,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/pdr.c:42:9: note: Assuming 'repo' is not equal to null
assert(repo != NULL);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:42:2: note: '?' condition is true
assert(repo != NULL);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:43:9: note: 'data' is not equal to null
assert(data != NULL);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:43:2: note: '?' condition is true
assert(data != NULL);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:44:9: note: 'size' is not equal to 0
assert(size != 0);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:44:2: note: '?' condition is true
assert(size != 0);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:47:9: note: Assuming 'record' is not equal to null
assert(record != NULL);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:47:2: note: '?' condition is true
assert(record != NULL);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:49:6: note: Assuming 'record_handle' is 0
if (record_handle) {
^~~~~~~~~~~~~
../src/pdr.c:49:2: note: Taking false branch
if (record_handle) {
^
../src/pdr.c:52:19: note: Assuming field 'last' is null
uint32_t curr = repo->last ? repo->last->record_handle : 0;
^~~~~~~~~~
../src/pdr.c:52:19: note: Assuming pointer value is null
uint32_t curr = repo->last ? repo->last->record_handle : 0;
^~~~~~~~~~
../src/pdr.c:52:19: note: '?' condition is false
../src/pdr.c:53:10: note: 'curr' is not equal to -1
assert(curr != UINT32_MAX);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:53:3: note: '?' condition is true
assert(curr != UINT32_MAX);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:60:6: note: 'data' is not equal to NULL
if (data != NULL) {
^~~~
../src/pdr.c:60:2: note: Taking true branch
if (data != NULL) {
^
../src/pdr.c:62:10: note: Assuming field 'data' is not equal to null
assert(record->data != NULL);
^
/usr/include/assert.h:95:7: note: expanded from macro 'assert'
((expr) \
^~~~
../src/pdr.c:62:3: note: '?' condition is true
assert(record->data != NULL);
^
/usr/include/assert.h:95:6: note: expanded from macro 'assert'
((expr) \
^
../src/pdr.c:70:8: note: 'record_handle' is 0
if (!record_handle) {
^~~~~~~~~~~~~
../src/pdr.c:70:3: note: Taking true branch
if (!record_handle) {
^
../src/pdr.c:78:6: note: Assuming field 'first' is not equal to NULL
if (repo->first == NULL) {
^~~~~~~~~~~~~~~~~~~
../src/pdr.c:78:2: note: Taking false branch
if (repo->first == NULL) {
^
../src/pdr.c:83:20: note: Access to field 'next' results in a dereference of a null pointer (loaded from field 'last')
repo->last->next = record;
~~~~ ^
```

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Ia54a3796ffa83804c23f913caed7d0943d1e7ad1

show more ...

pdr: Adjust condition to fix line wrap in get_new_record_handle()

C allows implicit conversion of pointers to booleans. Take advantage
of that by dropping the negation and explicit comparison to NUL

pdr: Adjust condition to fix line wrap in get_new_record_handle()

C allows implicit conversion of pointers to booleans. Take advantage
of that by dropping the negation and explicit comparison to NULL to
reduce the length of the statement.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: If606b5a934e521d6339f69a0145e292d5bdcdb96

show more ...

pdr: Inline add_record() and make_new_record() into pldm_pdr_add()

They only had a single call-site, in pldm_pdr_add(). Inlining the
functions improves readability by locality. The implementation do

pdr: Inline add_record() and make_new_record() into pldm_pdr_add()

They only had a single call-site, in pldm_pdr_add(). Inlining the
functions improves readability by locality. The implementation doesn't
reach excessive lengths, and it improves insight into the current use of
assert() for argument validation.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I825420b5b9f25685baf10c8b20675cf22f1c8ee1

show more ...

pdr: Introduce pldm_entity_association_pdr_add_from_node_check()

pldm_entity_association_pdr_add_from_node_check() returns whether or not
the operation was successful.
pldm_entity_association_pdr_ad

pdr: Introduce pldm_entity_association_pdr_add_from_node_check()

pldm_entity_association_pdr_add_from_node_check() returns whether or not
the operation was successful.
pldm_entity_association_pdr_add_from_node() is reimplemented in terms of
pldm_entity_association_pdr_add_from_node_check() with the intent that
it be deprecated once pldm_entity_association_pdr_add_from_node_check()
is stabilised.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I186c377bfc1038195540a6cdfd65db7e1126e869

show more ...