dsp: pdr: Rework test in pldm_entity_association_pdr_extract()
Testing on IBM's simulator[1] and bisection by Manoj
found that 9e566597d91e ("dsp: pdr: Bound check
pldm_entity_association_pdr_extract()") caused their host boot process
to come unstuck[1]:
[1]: https://gerrit.openbmc.org/c/openbmc/openbmc/+/75130/comments/fc6548ce_78285814
```
26.98655|2024/10/08 10:04:51|ISTEP 6. 5 - host_set_ipl_parms
27.02035|2024/10/08 10:04:51|ISTEP 6. 6 - host_discover_targets
27.31030|Detected new part : 00030000 (Physical:/Sys0/Node0/DIMM0)
27.64652|Detected new part : 00030004 (Physical:/Sys0/Node0/DIMM2)
29.58074|Detected new part : 00050000 (Physical:/Sys0/Node0/Proc0)
43.28986|Detected new part : 00050000 (Physical:/Sys0/Node0/Proc0)
47.49649|HWAS|---------------------------------
47.49650|HWAS|PRESENT>
47.52988|TARG|PROC=80000000
47.53612|TARG|PROC[00]:
47.53616|TARG| CORE=FF000000 DIMM=8800000000000000
47.53620|TARG| CACHE=FF000000 OCMB=C000
47.53632|TARG|PROC[01]:
47.53637|TARG| CORE=00000000 DIMM=0000000000000000
47.53641|TARG| CACHE=00000000 OCMB=0000
47.3653|TARG|PROC[02]:
47.53658|TARG| CORE=00000000 DIMM=0000000000000000
47.53662|TARG| CACHE=00000000 OCMB=0000
47.53674|TARG|PROC[03]:
47.53679|TARG| CORE=00000000 DIMM=0000000000000000
47.53683|TARG| CACHE=00000000 OCMB=0000
47.53738|WAS|---------------------------------
47.56477|devtree|Syncing to BMC
57.36060|Triggering graceful reboot fr Rebooting due to a FRU hot-remove
57.85530|================================================
57.87003|Error reprted by pldm (0x4700) EID 0x90000013
57.88170| Software problem, could not find reboot count effecter PDR.
57.88174| ModuleId 0x3e MOD_RESET_REBOOT_COUNT
57.90019| ReasonCode 0x471a RC_INVALID_EFFECTER_ID
57.90028| UserData1 The total number of PDRs that PDR Manager is aware of. : 0x00000000000001ee
57.90031| UserData2 : 0x0000000000000000
57.90034|------------------------------------------------
57.90038| Callouttype : Procedure Callout
57.90041| Procedure : EPUB_PRC_HB_CODE
57.92782| Priority : SRCI_PRIORITY_HIGH
57.92785|-----------------------------------------------
57.92788| Callout type : Procedure Callout
57.92791| Procedure : EPUB_PRC_SP_COE
57.92794| Priority : SRCI_PRIORITY_HIGH
57.92798|-----------------------------------------------
57.92801| Callout type : Procedure Callout
57.92804| Procedure : EPUB_PRC_HB_CODE
57.92808| Priority : SRCI_PRIORITY_MED
57.92811|------------------------------------------------
57.92816| Hostboot Build ID: hostboot-p11-e7437ab-sha1:997c6a7f/hbicore.bin
57.92819|================================================
59.00252|Soft poweroff requested by the BMC
```
This seems to be the result of the following from the BMC's side:
```
Oct 08 10:04:30 p10bmc pldmd[642]: Instance ID expiry for EID '9' using InstanceID '1'
Oct 08 10:04:30 p10bmc pldmd[642]: Failed to receive response for setEventReceiver command
Oct 08 10:04:32 p10bmc pldmd[642]: sdeventplus: ioCallback: Instance ID 1 for TID 9 was not previously allocated
Oct 08 10:04:32 p10bmc pldmd[642]: sdeventplus: ioCallback: Instance ID 1 for TID 9 was not previously allocated
Oct 08 10:04:32 p10bmc pldmd[642]: sdeventplus: ioCallback: Instance ID 1 for TID 9 was not previously allocated
Oct 08 10:04:32 p10bmc bmcwebd[282]: PAM unable to resolve symbol: pam_sm_acct_mgmt
Oct 08 10:04:33 p10bmc bmcwebd[282]: pam_succeed_if(webserver:auth): requirement "user ingroup redfish" was met by user "root"
Oct 08 10:04:35 p10bmc bmcwebd[282]: PAM unable to resolve symbol: pam_sm_acct_mgmt
Oct 08 10:04:35 p10bmc pldmd[642]: Checking if directory '/usr/share/pldm/pdr' exists
Oct 08 10:04:35 p10bmc pldmd[642]: Checking if directory '/usr/share/pldm/pdr/com.ibm.Hardware.Chassis.Model.Rainier4U' exists
Oct 08 10:04:35 p10bmc pldmd[642]: Failed to create sensor PDR, D-Bus object '/org/freedesktop/UPower/devices/ups_hiddev0' returned error - sd_bus_call: xyz.openbmc_project.Common.Error.ResourceNotFound: The resource is not found.
Oct 08 10:04:35 p10bmc bmcwebd[282]: pam_succeed_if(webserver:auth): requirement "user ingroup redfish" was met by user "root"
Oct 08 10:04:36 p10bmc pldmd[642]: Failed to create sensor PDR, D-Bus object '/xyz/openbmc_project/led/physical/virtual_enc_id' returned error - sd_bus_call: xyz.openbmc_project.Common.Error.ResourceNotFound: The resource is not found.
Oct 08 10:04:36 p10bmc pldmd[642]: Failed to create sensor PDR, D-Bus object '/xyz/openbmc_project/led/physical/virtual_enc_fault' returned error - sd_bus_call: xyz.openbmc_project.Common.Error.ResourceNotFound: The resource is not found.
```
Ultimately the check was trying to satisfy the following from GCC's
analyzer:
```
| 1345 | size_t l_num_entities = entity_association_pdr->num_children + 1;
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (16) ...to here
| 1346 | if (l_num_entities < 2) {
| | ~
| | |
| | (17) following ‘false’ branch (when ‘l_num_entities > 1’)...
|......
| 1354 | pldm_entity *l_entities = calloc(l_num_entities, sizeof(pldm_entity));
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (18) ...to here
| | (19) use of attacker-controlled value ‘(size_t)((int)*((char *)&*pdr + 10).num_children + 1) * 6’ as size without upper-bounds checking
|
```
However, rather than the relationship to pdr_len, the issue was the
confusion from the promotion from uint8_t to size_t. Teach the analyzer
about the UINT8_MAX limit explicitly.
gitlint-ignore: UC1, B1
Fixes: 9e566597d91e ("dsp: pdr: Bound check pldm_entity_association_pdr_extract()")
Change-Id: I0c71ba3b80da2946658a4e6a3add4636752b4e74
Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c5138dd..d357b6c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -172,6 +172,7 @@
15. dsp: bios: Bounds check encode_set_bios_table_req()
16. dsp: bios: Bounds check encode_set_bios_attribute_current_value_req()
17. dsp: bios_table: Bounds check pldm_bios_table_string_entry_encode()
+18. dsp: pdr: Rework test in pldm_entity_association_pdr_extract()
## [0.9.1] - 2024-09-07
diff --git a/src/dsp/pdr.c b/src/dsp/pdr.c
index 3a132a3..ccaa929 100644
--- a/src/dsp/pdr.c
+++ b/src/dsp/pdr.c
@@ -1341,17 +1341,13 @@
if (entity_association_pdr->num_children == UINT8_MAX) {
return;
}
+
size_t l_num_entities = entity_association_pdr->num_children + 1;
if (l_num_entities < 2) {
return;
}
- if (SIZE_MAX / sizeof(pldm_entity) < l_num_entities) {
- return;
- }
-
- if (pdr_len - sizeof(*entity_association_pdr) + 1 <
- sizeof(pldm_entity) * l_num_entities) {
+ if (UINT8_MAX < l_num_entities) {
return;
}