libpldmresponder: guard null host-path handlers pldmd only constructs hostPDRHandler and dbusToPLDMEventHandler when a host EID is configured, but three paths dereference them unconditionally, crashing pldmd on hostless platforms: - pldmPDRRepositoryChgEvent() marks and consumes hostPDRHandler for every PDR repository change event; guard those uses. - getStateSensorReadings() reads the sensor cache from dbusToPLDMEventHandler; fall back to an empty cache, so previousState reports PLDM_SENSOR_UNKNOWN as on a first read. - The OemIBM constructor wires the OEM handlers into hostPDRHandler; skip the wiring when the handler does not exist. Add regression tests for the two responder paths. The new event test surfaced a misaligned uint32_t read of the changeEntry array in getPDRRecordHandles(); copy each entry with memcpy instead. Change-Id: I5118ff9786254799790c848a68bdde5e6f6f2dfc Signed-off-by: JY Voon <jvoon@nvidia.com>
diff --git a/libpldmresponder/platform.cpp b/libpldmresponder/platform.cpp index 5a458d8..b42d9d6 100644 --- a/libpldmresponder/platform.cpp +++ b/libpldmresponder/platform.cpp
@@ -20,6 +20,7 @@ #include <phosphor-logging/lg2.hpp> +#include <cstring> #include <memory> PHOSPHOR_LOG2_USING; @@ -568,7 +569,7 @@ return rc; } - if (eventDataOperation == PLDM_RECORDS_MODIFIED) + if (hostPDRHandler && eventDataOperation == PLDM_RECORDS_MODIFIED) { hostPDRHandler->isHostPdrModified = true; } @@ -639,7 +640,12 @@ } for (size_t i = 0; i < numberOfChangeEntries; i++) { - pdrRecordHandles.push_back(changeEntryData[i]); + // the changeEntry array is offset by the PLDM message header and the + // event data framing in the request payload, so it is not naturally + // aligned for a direct read + ChangeEntry entry; + std::memcpy(&entry, changeEntryData + i, sizeof(ChangeEntry)); + pdrRecordHandles.push_back(entry); } return PLDM_SUCCESS; } @@ -811,10 +817,14 @@ } else { + static const stateSensorCacheMaps emptySensorCache{}; + const auto& sensorCache = dbusToPLDMEventHandler + ? dbusToPLDMEventHandler->getSensorCache() + : emptySensorCache; rc = platform_state_sensor::getStateSensorReadingsHandler< pldm::utils::DBusHandler, Handler>( dBusIntf, *this, sensorId, sensorRearmCount, comSensorCnt, - stateField, dbusToPLDMEventHandler->getSensorCache()); + stateField, sensorCache); } if (rc != PLDM_SUCCESS)
diff --git a/libpldmresponder/test/libpldmresponder_platform_test.cpp b/libpldmresponder/test/libpldmresponder_platform_test.cpp index bb0b8ac..05234bf 100644 --- a/libpldmresponder/test/libpldmresponder_platform_test.cpp +++ b/libpldmresponder/test/libpldmresponder_platform_test.cpp
@@ -922,3 +922,89 @@ pldm_pdr_destroy(inPDRRepo); pldm_pdr_destroy(outPDRRepo); } + +TEST(getStateSensorReadings, testNoDbusToPLDMEventHandler) +{ + // pldmd constructs dbusToPLDMEventHandler only when a host EID is + // configured; the responder must serve GetStateSensorReadings without it + std::array<uint8_t, + sizeof(pldm_msg_hdr) + PLDM_GET_STATE_SENSOR_READINGS_REQ_BYTES> + requestPayload{}; + auto req = std::start_lifetime_as<pldm_msg>(requestPayload.data()); + size_t requestPayloadLength = requestPayload.size() - sizeof(pldm_msg_hdr); + + bitfield8_t sensorRearm{}; + sensorRearm.byte = 0x01; + auto rc = encode_get_state_sensor_readings_req(0, 0x1, sensorRearm, 0, req); + ASSERT_EQ(rc, PLDM_SUCCESS); + + MockdBusHandler mockedUtils; + EXPECT_CALL(mockedUtils, getService(StrEq("/foo/bar"), _)) + .Times(1) + .WillRepeatedly(Return("foo.bar")); + + auto inPDRRepo = pldm_pdr_init(); + auto event = sdeventplus::Event::get_default(); + Handler handler(&mockedUtils, 0, nullptr, "./pdr_jsons/state_sensor/good", + inPDRRepo, nullptr, nullptr, nullptr, nullptr, nullptr, + event); + + auto response = handler.getStateSensorReadings(req, requestPayloadLength); + auto responsePtr = std::start_lifetime_as<pldm_msg>(response.data()); + + uint8_t completionCode{}; + uint8_t compSensorCnt{}; + std::array<get_sensor_state_field, 1> stateField{}; + rc = decode_get_state_sensor_readings_resp( + responsePtr, response.size() - sizeof(pldm_msg_hdr), &completionCode, + &compSensorCnt, stateField.data()); + ASSERT_EQ(rc, PLDM_SUCCESS); + EXPECT_EQ(completionCode, PLDM_SUCCESS); + ASSERT_EQ(compSensorCnt, 1); + // without the host event path there is no sensor cache, so the previous + // state is unknown as on a first read + EXPECT_EQ(stateField[0].previous_state, PLDM_SENSOR_UNKNOWN); + + pldm_pdr_destroy(inPDRRepo); +} + +TEST(pldmPDRRepositoryChgEvent, testNoHostPDRHandler) +{ + // pldmd constructs hostPDRHandler only when a host EID is configured; a + // terminus sending a repository change event with PLDM_RECORDS_MODIFIED + // must not crash the responder + std::array<uint8_t, 1> eventDataOps = {PLDM_RECORDS_MODIFIED}; + std::array<uint8_t, 1> numsOfChangeEntries = {1}; + std::array<uint32_t, 1> changeEntries = {1}; + const uint32_t* firstEntry = changeEntries.data(); + + size_t maxSize = PLDM_PDR_REPOSITORY_CHG_EVENT_MIN_LENGTH + + PLDM_PDR_REPOSITORY_CHANGE_RECORD_MIN_LENGTH + + changeEntries.size() * sizeof(uint32_t); + std::vector<uint8_t> requestPayload(sizeof(pldm_msg_hdr) + maxSize); + auto req = std::start_lifetime_as<pldm_msg>(requestPayload.data()); + auto eventData = std::start_lifetime_as<pldm_pdr_repository_chg_event_data>( + req->payload); + size_t actualSize{}; + auto rc = encode_pldm_pdr_repository_chg_event_data( + FORMAT_IS_PDR_HANDLES, 1, eventDataOps.data(), + numsOfChangeEntries.data(), &firstEntry, eventData, &actualSize, + maxSize); + ASSERT_EQ(rc, PLDM_SUCCESS); + + MockdBusHandler mockedUtils; + EXPECT_CALL(mockedUtils, getService(StrEq("/foo/bar"), _)) + .Times(1) + .WillRepeatedly(Return("foo.bar")); + + auto inPDRRepo = pldm_pdr_init(); + auto event = sdeventplus::Event::get_default(); + Handler handler(&mockedUtils, 0, nullptr, "./pdr_jsons/state_sensor/good", + inPDRRepo, nullptr, nullptr, nullptr, nullptr, nullptr, + event); + + rc = handler.pldmPDRRepositoryChgEvent(req, actualSize, 0x01, 1, 0); + EXPECT_EQ(rc, PLDM_SUCCESS); + + pldm_pdr_destroy(inPDRRepo); +}
diff --git a/pldmd/oem_ibm.hpp b/pldmd/oem_ibm.hpp index 0c5f0a6..be9a87b 100644 --- a/pldmd/oem_ibm.hpp +++ b/pldmd/oem_ibm.hpp
@@ -79,8 +79,11 @@ createOemPlatformHandler(); createOemIbmUtilsHandler(); codeUpdate->setOemPlatformHandler(oemPlatformHandler.get()); - hostPDRHandler->setOemPlatformHandler(oemPlatformHandler.get()); - hostPDRHandler->setOemUtilsHandler(oemUtilsHandler.get()); + if (hostPDRHandler) + { + hostPDRHandler->setOemPlatformHandler(oemPlatformHandler.get()); + hostPDRHandler->setOemUtilsHandler(oemUtilsHandler.get()); + } fruHandler->setOemPlatformHandler(oemPlatformHandler.get()); platformHandler->setOemPlatformHandler(oemPlatformHandler.get()); baseHandler->setOemPlatformHandler(oemPlatformHandler.get());