dsp: platform: Fix extra parentheses

No functionality change, avoids a warning.

gitlint-ignore: UC1
Fixes: 98e137dede32 ("dsp: platform: Fix decode_set_event_receiver_req()")
Change-Id: I38eae0d5e9

dsp: platform: Fix extra parentheses

No functionality change, avoids a warning.

gitlint-ignore: UC1
Fixes: 98e137dede32 ("dsp: platform: Fix decode_set_event_receiver_req()")
Change-Id: I38eae0d5e91c79e545b5bcca3a93bd4634435b62
Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>

show more ...

dsp: platform: Fix location of closing paren in overflow detection

I suspect this was the result of editor auto-parenthesis support and the
result got overlooked.

Add some tests while we're in the

dsp: platform: Fix location of closing paren in overflow detection

I suspect this was the result of editor auto-parenthesis support and the
result got overlooked.

Add some tests while we're in the area.

As seems to be the case when we expand the tests associated with
argument values, also update the ABI dump to reflect the change in
recorded register allocation.

gitlint-ignore: UC1
Fixes: #13
Fixes: ad33b99abcc4 ("dsp: platform: Bounds check encode_state_effecter_pdr()")
Reported-by: Daniel M. Crowell <dcrowell@us.ibm.com>
Change-Id: Iab4c1c337400678ac424936151a38baf0e0d554d
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKe

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKeepAlive.

Rework the change in 8c43abb due to the issue found in
openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting request
size")

gitlint-ignore: B1, UC1
Fixes: 66c7723adbdc ("msgbuf: Enable pldm_msgbuf_extract() into packed members")
Fixes: 9667f5823930 ("platform: pldm_msgbuf for decode_set_event_receiver_req()")
Fixes: 6ef2aa90a793 ("platform: Test invalid heartbeat conditions after assignment")
Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I7ca50e487b9f1e6c6ea2b34f73c363def8b2d295
Signed-off-by: Gilbert Chen <gilbertc@nvidia.com>

show more ...

Apply GCC's tainted_args attribute to library entrypoints

The implementation applies `__attribute__((tainted_args))` by
integrating it into the existing ABI macro annotations.

In the process, quite

Apply GCC's tainted_args attribute to library entrypoints

The implementation applies `__attribute__((tainted_args))` by
integrating it into the existing ABI macro annotations.

In the process, quite a number of APIs were discovered to be unsafe in
ways that were not immediately fixable. Often this is because they lack
arguments that enable the appropriate bounds-checking to be applied.

Redesigning them is work beyond the scope of the immediate
effort. Instead, we also introduce a new annotation,
LIBPLDM_ABI_DEPRECATED_UNSAFE, that simply lacks
`__attribute__((tainted_args))` and therefore doesn't trigger the extra
analysis.

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

show more ...

dsp: platform: Bounds check encode_state_effecter_pdr()

```
../src/dsp/platform.c:84:9: error: use of attacker-controlled value ‘possible_states_size’ as size without upper-bounds checking [CWE-129]

dsp: platform: Bounds check encode_state_effecter_pdr()

```
../src/dsp/platform.c:84:9: error: use of attacker-controlled value ‘possible_states_size’ as size without upper-bounds checking [CWE-129] [-Werror=analyzer-tainted-size]
84 | memcpy(effecter->possible_states, possible_states,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
85 | possible_states_size);
| ~~~~~~~~~~~~~~~~~~~~~
```

Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I7a53144c4c02639a0f7b7291277d8903d8f2717e
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Bounds check encode_sensor_state_pdr()

```
../src/dsp/platform.c: In function ‘encode_state_sensor_pdr’:
../src/dsp/platform.c:152:9: error: use of attacker-controlled value ‘possible

dsp: platform: Bounds check encode_sensor_state_pdr()

```
../src/dsp/platform.c: In function ‘encode_state_sensor_pdr’:
../src/dsp/platform.c:152:9: error: use of attacker-controlled value ‘possible_states_size’ as size without upper-bounds checking [CWE-129] [-Werror=analyzer-tainted-size]
152 | memcpy(sensor->possible_states, possible_states, possible_states_size);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: I682beae26d346e474825a393da7b5248d3166fbf
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Prevent overflow of arithmetic on event_data_length

Resolves the following warning from GCC's static analyzer:

```
../src/dsp/platform.c: In function ‘encode_platform_event_message_r

dsp: platform: Prevent overflow of arithmetic on event_data_length

Resolves the following warning from GCC's static analyzer:

```
../src/dsp/platform.c: In function ‘encode_platform_event_message_req’:
../src/dsp/platform.c:1246:9: error: use of attacker-controlled value ‘event_data_length’ as size without upper-bounds checking [CWE-129] [-Werror=analyzer-tainted-size]
1246 | memcpy(request->event_data, event_data, event_data_length);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: Id889a5b56d8403dea41f6acd43f21b44bf8d503d
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

dsp: platform: Merge PDR header extraction and validation

These two functions were tightly coupled, in the sense that in all
invocations of either were proximal to the other.

Merge them to avoid th

dsp: platform: Merge PDR header extraction and validation

These two functions were tightly coupled, in the sense that in all
invocations of either were proximal to the other.

Merge them to avoid the spatial separation and improve both our own and
the compiler's ability to reason about the relationships.

Change-Id: I098afac06fea56a26762879a620e74539f6e6d52
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 ...

Revert "dsp: platform: Fix decode_set_event_receiver_req()"

This reverts commit 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47.

As found in openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting
reque

Revert "dsp: platform: Fix decode_set_event_receiver_req()"

This reverts commit 8c43abb70aeadde39d99af2c1b6b5d4a1416fc47.

As found in openbmc/pldm@35f25949fe4d ("Fix invalid read by adjusting
request size") the change in 8c43abb70aea ("dsp: platform: Fix
decode_set_event_receiver_req()") reduces the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES and causes an out-of-bounds access.

A new macro should be introduced rather than changing the value of
PLDM_SET_EVENT_RECEIVER_REQ_BYTES.

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

show more ...

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKe

dsp: platform: Fix decode_set_event_receiver_req()

Per DSP0248 V1.3.0 table13, the heartbeatTimer field shall be omitted
from the request data if eventMessageGlobalEnable is not set to
enableAsyncKeepAlive.

gitlint-ignore: B1, UC1
Fixes: 66c7723adbdc ("msgbuf: Enable pldm_msgbuf_extract() into packed members")
Fixes: 9667f5823930 ("platform: pldm_msgbuf for decode_set_event_receiver_req()")
Fixes: 6ef2aa90a793 ("platform: Test invalid heartbeat conditions after assignment")
Fixes: 9c76679224cf ("libpldm: Migrate to subproject")
Change-Id: Ia2a0c71a1f37a0fd32670b9b66b051e18fb73dbe
Signed-off-by: Gilbert Chen <gilbertc@nvidia.com>

show more ...

platform: Fix checking `eventIDToAcknowledge`

As DSP0248 V1.3.0, `Figure 22 - Switching from asynchronous eventing to
poll for an event with large data` when the `tranferFlag` of
`PollForPlatformEve

platform: Fix checking `eventIDToAcknowledge`

As DSP0248 V1.3.0, `Figure 22 - Switching from asynchronous eventing to
poll for an event with large data` when the `tranferFlag` of
`PollForPlatformEventMessage` is `AcknowledgementOnly` the
`eventIDToAcknowledge` will be 0xffff.

But the contents in lines #1678 to #1681 of the same specs details
```
1678 0x0000 shall be returned to indicate the terminus event queue is empty. The PLDM Event Receiver shall
1679 acknowledge reception of the event by issuing the command again with the eventIDToAcknowledge set
1680 to the previously retrieved eventID (from the PLDM terminus). The PLDM terminus shall remove the
1681 acknowledged event message from its internal FIFO upon reception of the acknowledgment
```

When the `tranferFlag` is `AcknowledgementOnly` the
`eventIDToAcknowledge` should be `the previously retrieved eventID (from
the PLDM terminus)` which is not 0x0000 and 0xffff. Because `The PLDM
terminus shall remove the acknowledged event message` so the contents in
lines #1678 to #1681 are correct.

Update the failure condition in
`pldm_platform_poll_for_platform_event_message_validate` helper function
to follow the contents in lines #1678 to #1681. The helper will return
error when `tranferFlag` is `AcknowledgementOnly` and
`eventIDToAcknowledge` is 0xffff or 0x0000 (which can't be a real
previous event ID from terminus).

gitlint-ignore: B1, UC1
Fixes: 387b10f6cd37 ("platform: fix encode/decode_poll_for_platform_event_message_req")
Change-Id: Icf84494dbe2c43fba8ae2ec72e7197e8dd4abf3e
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Support PLDM_CPER_EVENT in encode_platform_event_message_req()

DSP0248 v1.3.0 add CPEREvent for PLDM Event Types as Table 11 and add
it in `encode_platform_event_message_req` func.

Change

platform: Support PLDM_CPER_EVENT in encode_platform_event_message_req()

DSP0248 v1.3.0 add CPEREvent for PLDM Event Types as Table 11 and add
it in `encode_platform_event_message_req` func.

Change-Id: I94e73938694ce8367489244b75c7e8e0a6d1d8ee
Signed-off-by: John Chung <john.chung@arm.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

platform: fix encode/decode_poll_for_platform_event_message_req

The checking `TransferOperationFlag` and `eventIDToAcknowledge` in
`encode/decode_poll_for_platform_event_message_req` APIs is not
cor

platform: fix encode/decode_poll_for_platform_event_message_req

The checking `TransferOperationFlag` and `eventIDToAcknowledge` in
`encode/decode_poll_for_platform_event_message_req` APIs is not
corrected. Update the conditions to follow the section `16.7
PollForPlatformEventMessage command` in DSP0248 V1.3.0.

Change-Id: Ie799d38f4a492b7719c2ff7c14fe83a1f81dc0b1
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Stabilise decode_pldm_platform_cper_event API

See usage example at:
[1] https://gerrit.openbmc.org/c/openbmc/pldm/+/63028/86/platform-mc/event_manager.cpp#77

Change-Id: I43c5c807f10ac38ee

platform: Stabilise decode_pldm_platform_cper_event API

See usage example at:
[1] https://gerrit.openbmc.org/c/openbmc/pldm/+/63028/86/platform-mc/event_manager.cpp#77

Change-Id: I43c5c807f10ac38ee893e65a9d75fca76312d188
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}

This allows us to elide checks where they're not necessary, and warn
people at compile-time when they're doing things they shouldn't.

Note that this c

compiler: Provide LIBPLDM_CC_NONNULL{,_ARGS()}

This allows us to elide checks where they're not necessary, and warn
people at compile-time when they're doing things they shouldn't.

Note that this comes with an apparent ABI break. abi-compliance-checker
reports:

```
platform.h, libpldm.so.0.8.0
[−] decode_sensor_op_data ( uint8_t const* sensor_data, size_t sensor_data_length, uint8_t* present_op_state, uint8_t* previous_op_state )
Change: The parameter previous_op_state became passed in r8 register instead of rcx.
Effect Applications will read the wrong memory block instead of the parameter value.
```

It's unclear to me why. The signature hasn't changed, but how the
implementation tests the parameter values has.

Change-Id: Ie8d8bc1641280522532d9b4764bf07c64b1921c8
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 ...

platform: Remove `_data` suffix from `cper_event` decode API

Rename `decode_pldm_platform_cper_event_data` to
`decode_pldm_platform_cper_event` as the API does not decode the CPER
data, it decodes t

platform: Remove `_data` suffix from `cper_event` decode API

Rename `decode_pldm_platform_cper_event_data` to
`decode_pldm_platform_cper_event` as the API does not decode the CPER
data, it decodes the event message.

Change-Id: I0937f043e4d3836f20733f78ea3f5970da6585d5
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Add decode_pldm_cper_event_data() API

Support decoder for `cperEvent` event class as table `Table 27 -
CPEREvent class eventData format` in DSP0248 V1.3.0.

Change-Id: I6165980e0570bbb2115

platform: Add decode_pldm_cper_event_data() API

Support decoder for `cperEvent` event class as table `Table 27 -
CPEREvent class eventData format` in DSP0248 V1.3.0.

Change-Id: I6165980e0570bbb21158af9e6adee15894b3bf3a
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Stabilise decode_pldm_message_poll_event_data() APIs

See usage example at:
[1] https://gerrit.openbmc.org/c/openbmc/pldm/+/73033/1/platform-mc/event_manager.cpp#126

Change-Id: Ic680a514b2

platform: Stabilise decode_pldm_message_poll_event_data() APIs

See usage example at:
[1] https://gerrit.openbmc.org/c/openbmc/pldm/+/73033/1/platform-mc/event_manager.cpp#126

Change-Id: Ic680a514b224d366cc00fa291f2da45dde77950a
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Rework decode/encode_pldm_message_poll_event_data() APIs

Adds `struct pldm_message_poll_event` which includes `format_version`,
`event_id` and `data_transfer_handle` in `pldmMessagePollEve

platform: Rework decode/encode_pldm_message_poll_event_data() APIs

Adds `struct pldm_message_poll_event` which includes `format_version`,
`event_id` and `data_transfer_handle` in `pldmMessagePollEvent` data.
Rework decode/encode_pldm_message_poll_event_data() APIs to use `struct
pldm_message_poll_event poll_event` as parameter.

Change-Id: I01c643ed320b17141d89068cb3e00b1a99162d77
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

platform: Stabilise related decode_entity_auxiliary_names_pdr*() APIs

See usage example at:
https://gerrit.openbmc.org/c/openbmc/pldm/+/72610

Change-Id: I16c6d42ad11afa59b04676fa187f5ea0764d1880
Si

platform: Stabilise related decode_entity_auxiliary_names_pdr*() APIs

See usage example at:
https://gerrit.openbmc.org/c/openbmc/pldm/+/72610

Change-Id: I16c6d42ad11afa59b04676fa187f5ea0764d1880
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>

show more ...

pdr: Add related decode_entity_auxiliary_names_pdr*() APIs

Add `decode_entity_auxiliary_names_pdr()` to decode the entity auxiliary
names PDR raw data to the data fields as table 95 in DSP0248_1.2.2

pdr: Add related decode_entity_auxiliary_names_pdr*() APIs

Add `decode_entity_auxiliary_names_pdr()` to decode the entity auxiliary
names PDR raw data to the data fields as table 95 in DSP0248_1.2.2.
The API will not decode the entity auxiliary names directly - to
expose the language tags and names fields the caller has to
subsequently call `decode_pldm_entity_auxiliary_names_pdr_index()`.
Between the API calls the caller must allocate memory for the
`names` field as an array of `struct pldm_entity_auxiliary_name` with
`name_string_count` elements.

Change-Id: I5fc3989c4c4595546a70c01eb2b6dadcf8c14303
Signed-off-by: Thu Nguyen <thu@os.amperecomputing.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

platform: Stabilise GetStateEffecterStates functions

See usage example at:
https://gerrit.openbmc.org/c/openbmc/pldm/+/71657

Change-Id: I8ef0961dbc8e87fb4ca8533142104b852a7fb0a4
Signed-off-by: Tal

platform: Stabilise GetStateEffecterStates functions

See usage example at:
https://gerrit.openbmc.org/c/openbmc/pldm/+/71657

Change-Id: I8ef0961dbc8e87fb4ca8533142104b852a7fb0a4
Signed-off-by: Tal Yacobi <talycb8@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...

platform: Change GetStateEffecterStates API error personality

APIs should return negative errno instead of PLDM completion codes.

Change-Id: Ic7017ecc02f3d969aa8fd5c6b1fa89dced484ab1
Signed-off-by:

platform: Change GetStateEffecterStates API error personality

APIs should return negative errno instead of PLDM completion codes.

Change-Id: Ic7017ecc02f3d969aa8fd5c6b1fa89dced484ab1
Signed-off-by: Tal Yacobi <talycb8@gmail.com>
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

show more ...