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>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2907ba3..55c6928 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -110,11 +110,6 @@
`tranferFlag` is `AcknowledgementOnly`, the value `eventIDToAcknowledge`
should be the previously retrieved eventID (from the PLDM terminus).
-4. dsp: platform: Fix decode_set_event_receiver_req()
-
- The heartbeat field shall be omitted if `eventMessageGlobalEnable` is not
- `enableAsyncKeepAlive`.
-
## [0.9.1] - 2024-09-07
### Changed
diff --git a/include/libpldm/platform.h b/include/libpldm/platform.h
index ed4ef3b..7ef48c7 100644
--- a/include/libpldm/platform.h
+++ b/include/libpldm/platform.h
@@ -29,13 +29,12 @@
};
/* Maximum size for request */
-#define PLDM_SET_STATE_EFFECTER_STATES_REQ_BYTES 19
-#define PLDM_GET_STATE_SENSOR_READINGS_REQ_BYTES 4
-#define PLDM_GET_NUMERIC_EFFECTER_VALUE_REQ_BYTES 2
-#define PLDM_GET_STATE_EFFECTER_STATES_REQ_BYTES 2
-#define PLDM_GET_SENSOR_READING_REQ_BYTES 3
-#define PLDM_SET_EVENT_RECEIVER_REQ_BYTES 3
-#define PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES 2
+#define PLDM_SET_STATE_EFFECTER_STATES_REQ_BYTES 19
+#define PLDM_GET_STATE_SENSOR_READINGS_REQ_BYTES 4
+#define PLDM_GET_NUMERIC_EFFECTER_VALUE_REQ_BYTES 2
+#define PLDM_GET_STATE_EFFECTER_STATES_REQ_BYTES 2
+#define PLDM_GET_SENSOR_READING_REQ_BYTES 3
+#define PLDM_SET_EVENT_RECEIVER_REQ_BYTES 5
/* Response lengths are inclusive of completion code */
#define PLDM_SET_STATE_EFFECTER_STATES_RESP_BYTES 1
diff --git a/src/dsp/platform.c b/src/dsp/platform.c
index a800da5..4b6898e 100644
--- a/src/dsp/platform.c
+++ b/src/dsp/platform.c
@@ -2429,16 +2429,9 @@
}
pldm_msgbuf_extract_p(buf, event_message_global_enable);
- if (rc) {
- return rc;
- }
-
pldm_msgbuf_extract_p(buf, transport_protocol_type);
pldm_msgbuf_extract_p(buf, event_receiver_address_info);
- if ((*event_message_global_enable ==
- PLDM_EVENT_MESSAGE_GLOBAL_ENABLE_ASYNC_KEEP_ALIVE)) {
- pldm_msgbuf_extract_p(buf, heartbeat_timer);
- }
+ pldm_msgbuf_extract_p(buf, heartbeat_timer);
rc = pldm_msgbuf_destroy(buf);
if (rc) {
diff --git a/tests/dsp/platform.cpp b/tests/dsp/platform.cpp
index bd615c3..e937dec 100644
--- a/tests/dsp/platform.cpp
+++ b/tests/dsp/platform.cpp
@@ -3539,8 +3539,10 @@
uint8_t eventReceiverAddressInfo = 0x08;
uint16_t heartbeatTimer = 0x78;
- PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+ std::vector<uint8_t> requestMsg(hdrSize +
+ PLDM_SET_EVENT_RECEIVER_REQ_BYTES);
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+ auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
auto rc = encode_set_event_receiver_req(
0, eventMessageGlobalEnable, transportProtocolType,
@@ -3564,8 +3566,10 @@
uint8_t eventReceiverAddressInfo = 0x08;
uint16_t heartbeatTimer = 0;
- PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+ std::vector<uint8_t> requestMsg(hdrSize +
+ PLDM_SET_EVENT_RECEIVER_REQ_BYTES);
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+ auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
auto rc = encode_set_event_receiver_req(
0, eventMessageGlobalEnable, transportProtocolType,
@@ -3635,8 +3639,9 @@
TEST(SetEventReceiver, testGoodDecodeRequest)
{
- PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+
+ std::array<uint8_t, hdrSize + PLDM_SET_EVENT_RECEIVER_REQ_BYTES>
+ requestMsg{};
uint8_t eventMessageGlobalEnable =
PLDM_EVENT_MESSAGE_GLOBAL_ENABLE_ASYNC_KEEP_ALIVE;
@@ -3644,6 +3649,8 @@
uint8_t eventReceiverAddressInfo = 0x08;
uint16_t heartbeatTimer = 0x78;
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+ auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
struct pldm_set_event_receiver_req* req =
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
reinterpret_cast<struct pldm_set_event_receiver_req*>(request->payload);
@@ -3658,11 +3665,9 @@
uint8_t reteventReceiverAddressInfo;
uint16_t retheartbeatTimer;
auto rc = decode_set_event_receiver_req(
- request,
- PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES,
- &reteventMessageGlobalEnable, &rettransportProtocolType,
- &reteventReceiverAddressInfo, &retheartbeatTimer);
+ request, requestMsg.size() - hdrSize, &reteventMessageGlobalEnable,
+ &rettransportProtocolType, &reteventReceiverAddressInfo,
+ &retheartbeatTimer);
EXPECT_EQ(rc, PLDM_SUCCESS);
EXPECT_EQ(eventMessageGlobalEnable, reteventMessageGlobalEnable);
@@ -3673,14 +3678,11 @@
TEST(SetEventReceiver, testBadDecodeRequest)
{
- PLDM_MSG_DEFINE_P(request, PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES);
+ std::array<uint8_t, hdrSize + PLDM_SET_EVENT_RECEIVER_REQ_BYTES>
+ requestMsg{};
- auto rc = decode_set_event_receiver_req(
- NULL,
- PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES,
- NULL, NULL, NULL, NULL);
+ auto rc = decode_set_event_receiver_req(NULL, requestMsg.size() - hdrSize,
+ NULL, NULL, NULL, NULL);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
uint8_t eventMessageGlobalEnable =
@@ -3689,6 +3691,8 @@
uint8_t eventReceiverAddressInfo = 0x08;
uint16_t heartbeatTimer = 0x78;
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
+ auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
struct pldm_set_event_receiver_req* req =
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
reinterpret_cast<struct pldm_set_event_receiver_req*>(request->payload);
@@ -3702,24 +3706,11 @@
uint8_t rettransportProtocolType;
uint8_t reteventReceiverAddressInfo;
uint16_t retheartbeatTimer;
-
-#ifdef NDEBUG
rc = decode_set_event_receiver_req(
- request,
- PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES - 1,
- &reteventMessageGlobalEnable, &rettransportProtocolType,
- &reteventReceiverAddressInfo, &retheartbeatTimer);
+ request, requestMsg.size() - hdrSize - 1, &reteventMessageGlobalEnable,
+ &rettransportProtocolType, &reteventReceiverAddressInfo,
+ &retheartbeatTimer);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
-#else
- EXPECT_DEATH(decode_set_event_receiver_req(
- request,
- PLDM_SET_EVENT_RECEIVER_REQ_BYTES +
- PLDM_SET_EVENT_RECEIVER_REQ_HEARTBEAT_BYTES - 1,
- &reteventMessageGlobalEnable, &rettransportProtocolType,
- &reteventReceiverAddressInfo, &retheartbeatTimer),
- "ctx->remaining >= 0");
-#endif
}
TEST(decodeNumericSensorPdrData, Uint8Test)