msgbuf: Rework error handling to improve soundness

Design the implementation to uphold the invariant that a non-negative
remaining value implies the cursor pointer is valid, and that under
other con

msgbuf: Rework error handling to improve soundness

Design the implementation to uphold the invariant that a non-negative
remaining value implies the cursor pointer is valid, and that under
other conditions error values must be observed by the msgbuf user. The
former is tested with assertions in the implementation. The latter is
enforced by construction.

With this change, all msgbuf instances for which
pldm_msgbuf_init_errno() succeeds must be either completed or discarded
by calls to the pldm_msgbuf_complete*() or pldm_msgbuf_discard() APIs
respectively.

We then build on the properties that:

- pldm_msgbuf_init_errno() is marked with the warn_unused_result
function attribute

- pldm_msgbuf_init_errno() returns errors for invalid buffer
configurations

- The complete and discard APIs are marked with the warn_unused_result
function attribute

- The complete APIs test for negative remaining values and return an
error if encountered.

- The discard API propagates the provided error code

Together these provide the foundation to ensure that buffer access
errors are (eventually) detected.

A msgbuf object is always in one of the uninitialized, valid, invalid,
or completed states. The states are defined as follows:

- Uninitialized: Undefined values for remaining and cursor

- Valid: cursor points to a valid object, remaining is both non-negative
and describes a range contained within the object pointed to
by cursor

- Invalid: The value of remaining is negative. The value of cursor is
unspecified.

- Completed: the value of remaining is INTMAX_MIN and cursor is NULL

msgbuf instances must always be in the completed state by the time
their storage is reclaimed. To enforce this, PLDM_MSGBUF_DEFINE_P()
is introduced both to simplify definition of related variables, and
to exploit the compiler's 'cleanup' attribute. The cleanup function
associated with the msgbuf object asserts that the referenced object is
in the completed state.

From there, update the implementations of the msgbuf APIs such that
exceeding implementation type limits forces the msgbuf object to the
invalid state (in addition to returning an error value) to relieve the
caller from testing the result of all API invocations.

Change-Id: I4d78ddc5f567d4148f2f6d8f3e7570e97c316bbb
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Rename 'destroy' APIs to 'complete'

Change the language to better reflect the intent, with the impending
introduction of the ability to 'discard' a msgbuf instance.

Change-Id: Idbb79dcc2587

msgbuf: Rename 'destroy' APIs to 'complete'

Change the language to better reflect the intent, with the impending
introduction of the ability to 'discard' a msgbuf instance.

Change-Id: Idbb79dcc2587a8baef67ffd405e0bc77e66fe995
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Initialize msgbuf after argument tests

The msgbuf APIs are being reworked to improve soundness and error
reporting. Prior to introducing some new requirements on its users,
ens

dsp: firmware_update: Initialize msgbuf after argument tests

The msgbuf APIs are being reworked to improve soundness and error
reporting. Prior to introducing some new requirements on its users,
ensure its init/destroy sequences have minimal code spans.

This effort surfaced a problem with a test configuration for
encode_get_downstream_firmware_parameters_req() which was passing
an invalid transfer operation flag. Previously this was masked by a
buffer-length validation failure.

Change-Id: I7259ac86d696da425ac9d919e6864dfb238d8996
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Wrap static errno variants of package APIs

Begin the process of migrating package parsing APIs away from using PLDM
protocol completion codes for signaling errors.

For now kee

dsp: firmware_update: Wrap static errno variants of package APIs

Begin the process of migrating package parsing APIs away from using PLDM
protocol completion codes for signaling errors.

For now keep the new symbols internal. They will eventually be exposed
and the current stable APIs deprecated.

Change-Id: Ieb67c43ebb0782b9da530c52de99b59edca4a648
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Stabilize downstream devices related ABI

These ABIs are now required by PLDM to query information from
downstream devices[1]. Mark them as stable to make the symbols
visible.

dsp: firmware_update: Stabilize downstream devices related ABI

These ABIs are now required by PLDM to query information from
downstream devices[1]. Mark them as stable to make the symbols
visible.

[1]: https://gerrit.openbmc.org/c/openbmc/pldm/+/75390

Change-Id: I6892fb6ffa878b8530e57ce338fe02048d9b08f7
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>

show more ...

dsp: firmware_update: Avoid integer overflow

A large fw_device_pkg_data_length could cause uint16_t
calc_min_record_length to wrap around. Instead use a size_t.

Change-Id: I1e0ee5a350d82cb477fd0955

dsp: firmware_update: Avoid integer overflow

A large fw_device_pkg_data_length could cause uint16_t
calc_min_record_length to wrap around. Instead use a size_t.

Change-Id: I1e0ee5a350d82cb477fd0955a11ded659a5c5933
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

dsp: Add FD side firmware_update encode/decode

This implements FD counterparts for firmware update (type 5)
encoding/decoding.

In tests after encoding a message, a subsequent decode is performed an

dsp: Add FD side firmware_update encode/decode

This implements FD counterparts for firmware update (type 5)
encoding/decoding.

In tests after encoding a message, a subsequent decode is performed and
the outputs are compared. This tests the FD portion of the message
decoding.

Change-Id: I5454acee19588b0679a9b0218588fc4c0a66b01d
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

dsp: firmware_update: Add iterator for downstream device parameters

The previous attempt where we invented a struct that made it possible to
hold full-sized version strings was awkward on several fr

dsp: firmware_update: Add iterator for downstream device parameters

The previous attempt where we invented a struct that made it possible to
hold full-sized version strings was awkward on several fronts. Replace
it with an iterator in the style of the downstream device descriptors.

Change-Id: If9b83f4704b3068de9113af7451051c086f39969
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Expand "params" in symbol names

Try to keep the names aligned with the spec so that they're more easily
searched for. We can abbreviate other words such as request, response,
l

dsp: firmware_update: Expand "params" in symbol names

Try to keep the names aligned with the spec so that they're more easily
searched for. We can abbreviate other words such as request, response,
length etc as necessary.

Change-Id: Ia5a2c93c153c70107be0fcddb1043b2e08cdd026
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: pack decomposed parameters to struct

There're two APIs that have decomposed parameters:
`encode_query_downstream_identifiers_req()` and
`encode_get_downstream_firmware_params_r

dsp: firmware_update: pack decomposed parameters to struct

There're two APIs that have decomposed parameters:
`encode_query_downstream_identifiers_req()` and
`encode_get_downstream_firmware_params_req(),
which against the checklist of API/ABI stabilization,
squashed those parameters to a struct to meet the request.

Change-Id: Ia952251cf8dcaeba060985e759e1d7aadf7b5b4d
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>

show more ...

dsp: firmware_update: Change return type of downstream device ABI/APIs

So far all of the downstream device related ABI/APIs were marked as
`TESTING`, before stabilize them, any deprecated code shoul

dsp: firmware_update: Change return type of downstream device ABI/APIs

So far all of the downstream device related ABI/APIs were marked as
`TESTING`, before stabilize them, any deprecated code should be
removed, including PLDM Completion Code, therefore, change all of the
return type of these to `ERRNO`.

Change-Id: Ie6b390fcc1c91a425f9181ec4ce4495729baab51
Signed-off-by: Unive Tien <unive.tien.wiwynn@gmail.com>

show more ...

dsp: firmware_update: Iterators for downstream device descriptors

Provide an ergonomic and safe means to iterate through downstream
devices and their descriptors while avoiding the need to allocate.

dsp: firmware_update: Iterators for downstream device descriptors

Provide an ergonomic and safe means to iterate through downstream
devices and their descriptors while avoiding the need to allocate. The
strategy leaves the library user to make their own choices about how the
data is handled.

The user-facing portion of the change takes the form of two new macros,
to be nested inside one another:

```
foreach_pldm_downstream_device(...) {
foreach_pldm_downstream_device_descriptor(...) {
// Do something with the device-specific descriptor
}
}
```

Examples uses are provided in the documentation and in changes to the
test suite.

Change-Id: I6e79454b94868da73f318635bcaae57cd51fbf97
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: firmware_update: Bounds check decode_downstream_device_parameter_table_entry_versions()

```
../src/dsp/firmware_update.c: In function ‘decode_downstream_device_parameter_table_entry_versions’:

dsp: firmware_update: Bounds check decode_downstream_device_parameter_table_entry_versions()

```
../src/dsp/firmware_update.c: In function ‘decode_downstream_device_parameter_table_entry_versions’:
../src/dsp/firmware_update.c:1248:48: error: use of attacker-controlled value ‘*entry.active_comp_ver_str_len’ as offset without upper-bounds checking [CWE-823] [-Werror=analyzer-tainted-offset]
1248 | active[entry->active_comp_ver_str_len] = '\0';
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
```

gitlint-ignore: T1, B1, UC1
Fixes: b6ef35b48065 ("fw_update: Add encode req & decode resp for get_downstream_fw_params")
Change-Id: I15571804f391dc97de6d80c90325ded006aee500
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Spec

msgbuf: Externalise error value conversion

We need to simplify the code to satisfy clang's analyzer, which seems
to struggle with assumptions if the code exceeds some unknown complexity
limit.

Specifically, this does away with pldm_msgbuf_init_cc() and all the
associated pldm_msgbuf_status() error translation machinery. All the
call-sites are fixed up, with some additional safety checks put in place
along the way.

I believe this change is viable because unless we're converting legacy
API implementations to use msgbuf there's no additional trickery, and
if we're converting existing implementations then care is required
regardless. The change of approach has no impact on implementation of
new APIs with msgbuf, as the current philosophy is that they should
return negative errnos anyway.

As seems to be the case with this kind of work, the parameter register
allocation seems to have been affected for a number of library APIs.
These are listed in the changelog, and the ABI dump has been updated.

Finally, for msgbuf use in the test cases, all instances have
been converted to use errnos in place of PLDM completion codes in the
expectations. Hopefully there's no more malarky with PLDM completion
code misuse in the future.

Change-Id: Id4a7366ee9f60fb991dfe84aa0bb5aadc9855fcc
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

msgbuf: Harden pldm_msgbuf_{insert,extract}_array()

Review of some proposed APIs suggested that correct use of the
pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it
should be.

msgbuf: Harden pldm_msgbuf_{insert,extract}_array()

Review of some proposed APIs suggested that correct use of the
pldm_msgbuf_{insert,extract}_array() helpers was more difficult that it
should be. In the three-parameter form, it was too tempting to provide
the length to extract as parsed out of a PLDM message. The intended
use was that the length parameter represented the length of the
user-provided data buffer.

Instead, move to a four-parameter form, provide reasonable documentation
for how these APIs should be used, fix all the call-sites, and deprecate
some existing unsafe APIs.

Change-Id: If58e5574600e80b354f383554283c4eda5d7234c
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

fw_update: Add encode req & decode resp for get_downstream_fw_params

Add support for Get Downstream Firmware Parameters to ask all
downstream devices' Firmware Parameters.

The code is developed bas

fw_update: Add encode req & decode resp for get_downstream_fw_params

Add support for Get Downstream Firmware Parameters to ask all
downstream devices' Firmware Parameters.

The code is developed based on the definition of
'GetDownstreamFirmwareParameters' in DSP0267_1.1.0. Section 10.5

Change-Id: I291ca3b623be6119434b70494bb9a12b22f600b9
Signed-off-by: Chris Wang <chris.wang.wiwynn@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

Fix spelling mistakes using codespell

This commit corrects various spelling mistakes throughout the
repository. The corrections were made automatically using `codespell`[1]
tool.

[1]: https://githu

Fix spelling mistakes using codespell

This commit corrects various spelling mistakes throughout the
repository. The corrections were made automatically using `codespell`[1]
tool.

[1]: https://github.com/codespell-project/codespell

Change-Id: I25415165df192cfc3bd1405aca81bfa5bf2f7a63
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>

show more ...

libpldm: Reorganize source and test files

Primarily this is about moving specification-specific files into
'dsp/' (in the "DMTF Standard Publication" sense[1]) subdirectories of
both src/ and tests/

libpldm: Reorganize source and test files

Primarily this is about moving specification-specific files into
'dsp/' (in the "DMTF Standard Publication" sense[1]) subdirectories of
both src/ and tests/.

[1]: https://www.dmtf.org/sites/default/files/standards/documents/DSP4014_2.14.0.pdf

libpldm is a concrete C implementation of the PLDM family of
specifications. This invokes some accidental complexity[2] such as the
msgbuf APIs and other concerns.

[2]: https://en.wikipedia.org/wiki/No_Silver_Bullet

Separate the essential complexity (everything under the dsp/
subdirectories) from the accidental complexity (almost everything else).

While doing so, I took the opportunity to drop the 'libpldm_' prefix
and '_test' suffix from a variety of tests. The 'libpldm_' prefix is a
hangover from the days when libpldm was a subproject of OpenBMC's pldm
repo. The '_test' suffix feels redundant given the parent directory
path.

Note that we maintain separation of the src/ and tests/. The test suite
is implemented in C++ while libpldm's APIs are declared and defined in
C. The ability to chop all the tests and C++ out of the implementation
by ignoring a subtree seems like a desirable property when vendoring the
library into other projects.

Finally, update the x86_64 GCC ABI dump, as rearranging the source
causes a lot of churn in its definitions.

Change-Id: Icffcc6cf48b3101ecd38168827c0a81cffb8f083
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...