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>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 96ee811..dcdd2fc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -27,7 +27,7 @@
6. pldm_entity_association_tree_copy_root_check()
7. oem: ibm: Add topology related state set and enum
-7. base: Add size and buffer macros for struct pldm_msg
+8. base: Add size and buffer macros for struct pldm_msg
Together these macros reduce the need for use of reinterpret_cast<>() in C++.
@@ -97,6 +97,11 @@
Avoid a caller-controlled NULL pointer dereference in the library
implementation.
+2. platform: fix encode/decode_poll_for_platform_event_message_req
+
+ Update checking of `TransferOperationFlag` and `eventIDToAcknowledge` to
+ follow spec.
+
## [0.9.1] - 2024-09-07
### Changed
diff --git a/include/libpldm/platform.h b/include/libpldm/platform.h
index 5f23983..7ef48c7 100644
--- a/include/libpldm/platform.h
+++ b/include/libpldm/platform.h
@@ -52,11 +52,19 @@
#define PLDM_EVENT_MESSAGE_SUPPORTED_REQ_BYTES 1
#define PLDM_EVENT_MESSAGE_SUPPORTED_MIN_RESP_BYTES 4
+/* PollForPlatformEventMessage PLDM request */
#define PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES 8
#define PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_MIN_RESP_BYTES 4
#define PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_RESP_BYTES 14
#define PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_CHECKSUM_BYTES 4
+/* Platform event message request */
+#define PLDM_PLATFORM_EVENT_ID_NULL 0x0000
+#define PLDM_PLATFORM_EVENT_ID_FRAGMENT 0xffff
+/* Platform event message response */
+#define PLDM_PLATFORM_EVENT_ID_NONE 0x0000
+#define PLDM_PLATFORM_EVENT_ID_ACK 0xffff
+
/* Minimum response length */
#define PLDM_GET_PDR_MIN_RESP_BYTES 12
#define PLDM_GET_NUMERIC_EFFECTER_VALUE_MIN_RESP_BYTES 5
diff --git a/src/dsp/platform.c b/src/dsp/platform.c
index 76b8992..4ecad13 100644
--- a/src/dsp/platform.c
+++ b/src/dsp/platform.c
@@ -1046,6 +1046,22 @@
return pldm_msgbuf_destroy(buf);
}
+static int pldm_platform_poll_for_platform_event_message_validate(
+ uint8_t transfer_operation_flag, uint16_t event_id_to_acknowledge)
+{
+ if (((transfer_operation_flag == PLDM_GET_FIRSTPART) &&
+ (event_id_to_acknowledge != PLDM_PLATFORM_EVENT_ID_NULL)) ||
+ ((transfer_operation_flag == PLDM_GET_NEXTPART) &&
+ (event_id_to_acknowledge != PLDM_PLATFORM_EVENT_ID_FRAGMENT)) ||
+ ((transfer_operation_flag == PLDM_ACKNOWLEDGEMENT_ONLY) &&
+ (event_id_to_acknowledge != PLDM_PLATFORM_EVENT_ID_FRAGMENT)) ||
+ (transfer_operation_flag > PLDM_ACKNOWLEDGEMENT_ONLY)) {
+ return -EPROTO;
+ }
+
+ return 0;
+}
+
LIBPLDM_ABI_STABLE
int decode_poll_for_platform_event_message_req(
const struct pldm_msg *msg, size_t payload_length,
@@ -1084,11 +1100,9 @@
return rc;
}
- if (!(((*transfer_operation_flag == PLDM_GET_NEXTPART) &&
- (*event_id_to_acknowledge == 0xffff)) ||
- ((*transfer_operation_flag == PLDM_GET_FIRSTPART) &&
- (*event_id_to_acknowledge == 0x000)) ||
- (*transfer_operation_flag == PLDM_ACKNOWLEDGEMENT_ONLY))) {
+ rc = pldm_platform_poll_for_platform_event_message_validate(
+ *transfer_operation_flag, *event_id_to_acknowledge);
+ if (rc < 0) {
return PLDM_ERROR_INVALID_DATA;
}
@@ -2473,6 +2487,12 @@
return PLDM_ERROR_INVALID_DATA;
}
+ rc = pldm_platform_poll_for_platform_event_message_validate(
+ transfer_operation_flag, event_id_to_acknowledge);
+ if (rc < 0) {
+ return PLDM_ERROR_INVALID_DATA;
+ }
+
struct pldm_header_info header = { 0 };
header.msg_type = PLDM_REQUEST;
header.instance = instance_id;
diff --git a/tests/dsp/platform.cpp b/tests/dsp/platform.cpp
index a8a530c..f94e5f7 100644
--- a/tests/dsp/platform.cpp
+++ b/tests/dsp/platform.cpp
@@ -1397,18 +1397,14 @@
EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
}
-TEST(PollForPlatformEventMessage, testGoodEncodeRequest)
+TEST(PollForPlatformEventMessage, testGoodEncodeRequestFirstPart)
{
uint8_t formatVersion = 0x01;
- uint8_t transferOperationFlag = 0x1;
- uint32_t dataTransferHandle = 0xffffffff;
- uint16_t eventIdToAcknowledge = 0x0;
+ uint8_t transferOperationFlag = PLDM_GET_FIRSTPART;
+ uint32_t dataTransferHandle = 0xaabbccdd;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_NULL;
- std::array<uint8_t,
- hdrSize + PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES>
- requestMsg{};
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
+ PLDM_MSG_DEFINE_P(request, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
auto rc = encode_poll_for_platform_event_message_req(
0, formatVersion, transferOperationFlag, dataTransferHandle,
@@ -1418,10 +1414,10 @@
struct pldm_msgbuf _buf;
struct pldm_msgbuf* buf = &_buf;
- rc = pldm_msgbuf_init_cc(
+ rc = pldm_msgbuf_init_errno(
buf, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES, request->payload,
PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
- EXPECT_EQ(rc, PLDM_SUCCESS);
+ EXPECT_EQ(rc, 0);
uint8_t retFormatVersion;
uint8_t retTransferOperationFlag;
@@ -1432,37 +1428,154 @@
pldm_msgbuf_extract_uint8(buf, &retTransferOperationFlag);
pldm_msgbuf_extract_uint32(buf, &retDataTransferHandle);
pldm_msgbuf_extract_uint16(buf, &retEventIdToAcknowledge);
+ ASSERT_EQ(pldm_msgbuf_destroy_consumed(buf), 0);
EXPECT_EQ(retFormatVersion, formatVersion);
EXPECT_EQ(retTransferOperationFlag, transferOperationFlag);
EXPECT_EQ(retDataTransferHandle, dataTransferHandle);
EXPECT_EQ(retEventIdToAcknowledge, eventIdToAcknowledge);
- EXPECT_EQ(pldm_msgbuf_destroy(buf), PLDM_SUCCESS);
+}
+
+TEST(PollForPlatformEventMessage, testGoodEncodeRequestNextPart)
+{
+ uint8_t formatVersion = 0x01;
+ uint8_t transferOperationFlag = PLDM_GET_NEXTPART;
+ uint32_t dataTransferHandle = 0xaabbccdd;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
+
+ PLDM_MSG_DEFINE_P(request, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+
+ auto rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_SUCCESS);
+
+ struct pldm_msgbuf _buf;
+ struct pldm_msgbuf* buf = &_buf;
+ rc = pldm_msgbuf_init_errno(
+ buf, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES, request->payload,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, 0);
+
+ uint8_t retFormatVersion;
+ uint8_t retTransferOperationFlag;
+ uint32_t retDataTransferHandle;
+ uint16_t retEventIdToAcknowledge;
+
+ pldm_msgbuf_extract_uint8(buf, &retFormatVersion);
+ pldm_msgbuf_extract_uint8(buf, &retTransferOperationFlag);
+ pldm_msgbuf_extract_uint32(buf, &retDataTransferHandle);
+ pldm_msgbuf_extract_uint16(buf, &retEventIdToAcknowledge);
+ ASSERT_EQ(pldm_msgbuf_destroy_consumed(buf), 0);
+
+ EXPECT_EQ(retFormatVersion, formatVersion);
+ EXPECT_EQ(retTransferOperationFlag, transferOperationFlag);
+ EXPECT_EQ(retDataTransferHandle, dataTransferHandle);
+ EXPECT_EQ(retEventIdToAcknowledge, eventIdToAcknowledge);
+}
+
+TEST(PollForPlatformEventMessage, testGoodEncodeRequestAckOnly)
+{
+ uint8_t formatVersion = 0x01;
+ uint8_t transferOperationFlag = PLDM_ACKNOWLEDGEMENT_ONLY;
+ uint32_t dataTransferHandle = 0xaabbccdd;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
+
+ PLDM_MSG_DEFINE_P(request, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+
+ auto rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_SUCCESS);
+
+ struct pldm_msgbuf _buf;
+ struct pldm_msgbuf* buf = &_buf;
+ rc = pldm_msgbuf_init_errno(
+ buf, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES, request->payload,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, 0);
+
+ uint8_t retFormatVersion;
+ uint8_t retTransferOperationFlag;
+ uint32_t retDataTransferHandle;
+ uint16_t retEventIdToAcknowledge;
+
+ pldm_msgbuf_extract_uint8(buf, &retFormatVersion);
+ pldm_msgbuf_extract_uint8(buf, &retTransferOperationFlag);
+ pldm_msgbuf_extract_uint32(buf, &retDataTransferHandle);
+ pldm_msgbuf_extract_uint16(buf, &retEventIdToAcknowledge);
+ ASSERT_EQ(pldm_msgbuf_destroy_consumed(buf), 0);
+
+ EXPECT_EQ(retFormatVersion, formatVersion);
+ EXPECT_EQ(retTransferOperationFlag, transferOperationFlag);
+ EXPECT_EQ(retDataTransferHandle, dataTransferHandle);
+ EXPECT_EQ(retEventIdToAcknowledge, eventIdToAcknowledge);
}
TEST(PollForPlatformEventMessage, testBadEncodeRequest)
{
uint8_t formatVersion = 0x01;
- uint8_t transferOperationFlag = 0x1;
- uint32_t dataTransferHandle = 0xffffffff;
- uint16_t eventIdToAcknowledge = 0x0;
+ uint8_t transferOperationFlag = PLDM_GET_FIRSTPART;
+ uint32_t dataTransferHandle = 0xaabbccdd;
+ uint16_t eventIdToAcknowledge = 0x1234;
- std::array<uint8_t,
- hdrSize + PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES>
- requestMsg{};
- // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
- auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
+ PLDM_MSG_DEFINE_P(request, PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
auto rc = encode_poll_for_platform_event_message_req(
0, formatVersion, transferOperationFlag, dataTransferHandle,
eventIdToAcknowledge, nullptr,
PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
-
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
- encode_poll_for_platform_event_message_req(
+ transferOperationFlag = PLDM_GET_FIRSTPART;
+ eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
+ rc = encode_poll_for_platform_event_message_req(
0, formatVersion, transferOperationFlag, dataTransferHandle,
- eventIdToAcknowledge, request, hdrSize);
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ transferOperationFlag = PLDM_GET_NEXTPART;
+ eventIdToAcknowledge = 0x1234;
+ rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ transferOperationFlag = PLDM_GET_NEXTPART;
+ eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_NULL;
+ rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ transferOperationFlag = PLDM_ACKNOWLEDGEMENT_ONLY;
+ eventIdToAcknowledge = 0x1234;
+ rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ transferOperationFlag = PLDM_ACKNOWLEDGEMENT_ONLY;
+ eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_NULL;
+ rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+ transferOperationFlag = PLDM_ACKNOWLEDGEMENT_ONLY + 1;
+ eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
+ rc = encode_poll_for_platform_event_message_req(
+ 0, formatVersion, transferOperationFlag, dataTransferHandle,
+ eventIdToAcknowledge, request,
+ PLDM_POLL_FOR_PLATFORM_EVENT_MESSAGE_REQ_BYTES);
+ EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
}
TEST(PollForPlatformEventMessage, testGoodDecodeRespond)
@@ -1659,7 +1772,7 @@
uint8_t formatVersion = 0x1;
uint8_t transferOperationFlag = PLDM_GET_FIRSTPART;
uint32_t dataTransferHandle = 0x11223344;
- uint16_t eventIdToAcknowledge = 0x0;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_NULL;
std::vector<uint8_t> requestMsg{0x1, 0x0, 0x0, 0x1, PLDM_GET_FIRSTPART,
0x44, 0x33, 0x22, 0x11, 0x00,
0x00};
@@ -1688,7 +1801,7 @@
uint8_t formatVersion = 0x1;
uint8_t transferOperationFlag = PLDM_GET_NEXTPART;
uint32_t dataTransferHandle = 0x11223344;
- uint16_t eventIdToAcknowledge = 0xffff;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
std::vector<uint8_t> requestMsg{0x1, 0x0, 0x0, 0x1, PLDM_GET_NEXTPART,
0x44, 0x33, 0x22, 0x11, 0xff,
0xff};
@@ -1717,7 +1830,7 @@
uint8_t formatVersion = 0x1;
uint8_t transferOperationFlag = PLDM_ACKNOWLEDGEMENT_ONLY;
uint32_t dataTransferHandle = 0x11223344;
- uint16_t eventIdToAcknowledge = 0xffff;
+ uint16_t eventIdToAcknowledge = PLDM_PLATFORM_EVENT_ID_FRAGMENT;
std::vector<uint8_t> requestMsg{
0x1, 0x0, 0x0, 0x1, PLDM_ACKNOWLEDGEMENT_ONLY, 0x44, 0x33,
0x22, 0x11, 0xff, 0xff};
@@ -1743,6 +1856,10 @@
TEST(PollForPlatformEventMessage, testBadDecodeRequest)
{
+ /*
+ * transfer_operation_flag is PLDM_GET_FIRSTPART and
+ * event_id_to_acknowledge is not PLDM_PLATFORM_EVENT_ID_NULL
+ */
std::vector<uint8_t> requestMsg{0x1, 0x0, 0x0, 0x1, PLDM_GET_FIRSTPART,
0x44, 0x33, 0x22, 0x11, 0x66,
0x55};
@@ -1773,43 +1890,14 @@
&retEventIdToAcknowledge);
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
- /*
- * transfer_operation_flag is PLDM_GET_FIRSTPART and
- * event_id_to_acknowledge not 0x0000
- */
- requestMsg[4] = PLDM_GET_FIRSTPART;
- requestMsg[9] = 0x0;
- requestMsg[10] = 0x1;
-
- rc = decode_poll_for_platform_event_message_req(
- request, requestMsg.size() - hdrSize, &retFormatVersion,
- &retTransferOperationFlag, &retDataTransferHandle,
- &retEventIdToAcknowledge);
-
- EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
-
- /*
- * transfer_operation_flag is not PLDM_GET_FIRSTPART and
- * event_id_to_acknowledge is 0x0000
- */
- requestMsg[4] = PLDM_GET_NEXTPART;
- requestMsg[9] = 0x0;
- requestMsg[10] = 0x0;
-
- rc = decode_poll_for_platform_event_message_req(
- request, requestMsg.size() - hdrSize, &retFormatVersion,
- &retTransferOperationFlag, &retDataTransferHandle,
- &retEventIdToAcknowledge);
-
- EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
/*
* transfer_operation_flag is PLDM_GET_NEXTPART and
- * event_id_to_acknowledge not 0xffff
+ * event_id_to_acknowledge is not PLDM_PLATFORM_EVENT_ID_FRAGMENT
*/
requestMsg[4] = PLDM_GET_NEXTPART;
- requestMsg[9] = 0x0;
- requestMsg[10] = 0x1;
+ requestMsg[9] = 0x11;
+ requestMsg[10] = 0x22;
rc = decode_poll_for_platform_event_message_req(
request, requestMsg.size() - hdrSize, &retFormatVersion,
@@ -1819,12 +1907,12 @@
EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
/*
- * transfer_operation_flag is not PLDM_GET_NEXTPART and
- * event_id_to_acknowledge is 0xffff
+ * transfer_operation_flag is PLDM_ACKNOWLEDGEMENT_ONLY and
+ * event_id_to_acknowledge is not PLDM_PLATFORM_EVENT_ID_FRAGMENT
*/
- requestMsg[4] = PLDM_GET_FIRSTPART;
- requestMsg[9] = 0xff;
- requestMsg[10] = 0xff;
+ requestMsg[4] = PLDM_ACKNOWLEDGEMENT_ONLY;
+ requestMsg[9] = 0x11;
+ requestMsg[10] = 0x22;
rc = decode_poll_for_platform_event_message_req(
request, requestMsg.size() - hdrSize, &retFormatVersion,