dsp: base: Don't extract MultipartReceive resp's CRC once complete
According to DSP0240 v1.2.0 - Table 17 MultipartReceive Request and
Response Message Format, `DataIntegrityChecksum` field:
- Shall be included with all part transfers.
- Shall be omitted in response to a request message where
TransferOperation was set to XFER_COMPLETE or XFER_ABORT.
The only possible TransferFlag in a response for a request, where
TransferOperation was set to `XFER_COMPLETE` or `XFER_ABORT`, is
`ACKNOWLEDGE_COMPLETION`. So this is the only condition to ignore the
`DataIntegrityChecksum` field.
In the current implementation, `DataIntegrityChecksum` field is only
extracted when `TransferOperation` is `END` or `START_AND_END` and this
is incorrect.
Therefore, this commit updates `decode_pldm_base_multipart_receive_resp`
function to not extract `DataIntegrityChecksum` only when `TransferFlag`
field is `ACKNOWLEDGE_COMPLETION`. This also adds 2 more unit tests for
the API, which are respectively decoding an `ACKNOWLEDGE_COMPLETION`
response with a redundant checksum field and decoding a
non-`ACKNOWLEDGE_COMPLETION` response without a checksum field.
Change-Id: I325a6393eaabfecc08f758dcae417470dde65efb
Fixes: 8836784f1e06 ("dsp: base: Add encode req & decode resp for MultipartReceive command")
Signed-off-by: Chau Ly <chaul@amperecomputing.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8fe6cb1..ba9f375 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -73,6 +73,8 @@
### Fixed
+- dsp: base: Don't extract MultipartReceive resp's CRC once complete
+
### Security
## [0.14.0] 2025-08-11
diff --git a/src/dsp/base.c b/src/dsp/base.c
index 7ca3e2b..05fcb9a 100644
--- a/src/dsp/base.c
+++ b/src/dsp/base.c
@@ -666,10 +666,8 @@
(void **)&resp->data.ptr);
}
- if (resp->transfer_flag ==
- PLDM_BASE_MULTIPART_RECEIVE_TRANSFER_FLAG_END ||
- resp->transfer_flag ==
- PLDM_BASE_MULTIPART_RECEIVE_TRANSFER_FLAG_START_AND_END) {
+ if (resp->transfer_flag !=
+ PLDM_BASE_MULTIPART_RECEIVE_TRANSFER_FLAG_ACK_COMPLETION) {
pldm_msgbuf_extract_p(buf, data_integrity_checksum);
}
diff --git a/tests/dsp/base.cpp b/tests/dsp/base.cpp
index 723810e..890c74a 100644
--- a/tests/dsp/base.cpp
+++ b/tests/dsp/base.cpp
@@ -968,6 +968,106 @@
#endif
#ifdef LIBPLDM_API_TESTING
+TEST(DecodeMultipartReceiveResponse, BadTestRedundantCheckSum)
+{
+ uint8_t completionCode = PLDM_SUCCESS;
+ uint8_t transferFlag =
+ PLDM_BASE_MULTIPART_RECEIVE_TRANSFER_FLAG_ACK_COMPLETION;
+ uint32_t nextDataTransferHandle = 0x00;
+ static constexpr const uint32_t dataLength = 0;
+ uint32_t dataIntegrityChecksum = 0x2c;
+
+ struct pldm_base_multipart_receive_resp resp_data = {};
+
+ PLDM_MSGBUF_DEFINE_P(buf);
+ int rc;
+
+ /*
+ * Data field is omitted in a response with ACKNOWLEDGE_COMPLETION
+ * TransferFlag. Intentionally insert a DataIntegrityChecksum field to the
+ * response message
+ */
+
+ static constexpr const size_t payload_length =
+ PLDM_BASE_MULTIPART_RECEIVE_RESP_MIN_BYTES + dataLength +
+ sizeof(dataIntegrityChecksum);
+ PLDM_MSG_DEFINE_P(responseMsg, payload_length);
+
+ rc = pldm_msgbuf_init_errno(buf, 0, responseMsg->payload, payload_length);
+ ASSERT_EQ(rc, 0);
+
+ pldm_msgbuf_insert_uint8(buf, completionCode);
+ pldm_msgbuf_insert_uint8(buf, transferFlag);
+ pldm_msgbuf_insert_uint32(buf, nextDataTransferHandle);
+ pldm_msgbuf_insert_uint32(buf, dataLength);
+ pldm_msgbuf_insert_uint32(buf, dataIntegrityChecksum);
+
+ ASSERT_EQ(pldm_msgbuf_complete_consumed(buf), 0);
+
+ uint32_t respDataIntegrityChecksum = 0;
+
+ rc = decode_pldm_base_multipart_receive_resp(
+ responseMsg, payload_length, &resp_data, &respDataIntegrityChecksum);
+
+ /*
+ * pldm_msgbuf_complete_consumed() returns -EBADMSG as
+ * decode_pldm_base_multipart_receive_resp() didn't consume all the input
+ * message buffer.
+ */
+ EXPECT_EQ(rc, -EBADMSG);
+}
+#endif
+
+#ifdef LIBPLDM_API_TESTING
+TEST(DecodeMultipartReceiveResponse, BadTestMissingCheckSum)
+{
+ uint8_t completionCode = PLDM_SUCCESS;
+ uint8_t transferFlag = PLDM_BASE_MULTIPART_RECEIVE_TRANSFER_FLAG_END;
+ uint32_t nextDataTransferHandle = 0x00;
+ static constexpr const uint32_t dataLength = 9;
+ std::vector<uint8_t> data = {1, 2, 3, 4, 5, 6, 7, 8, 9};
+
+ struct pldm_base_multipart_receive_resp resp_data = {};
+
+ PLDM_MSGBUF_DEFINE_P(buf);
+ int rc;
+
+ /*
+ * Intentionally do not insert DataIntegrityChecksum field to the response
+ * message
+ */
+ static constexpr const size_t payload_length =
+ PLDM_BASE_MULTIPART_RECEIVE_RESP_MIN_BYTES + dataLength;
+ PLDM_MSG_DEFINE_P(responseMsg, payload_length);
+
+ rc = pldm_msgbuf_init_errno(buf, 0, responseMsg->payload, payload_length);
+ ASSERT_EQ(rc, 0);
+
+ pldm_msgbuf_insert_uint8(buf, completionCode);
+ pldm_msgbuf_insert_uint8(buf, transferFlag);
+ pldm_msgbuf_insert_uint32(buf, nextDataTransferHandle);
+ pldm_msgbuf_insert_uint32(buf, dataLength);
+ rc = pldm_msgbuf_insert_array_uint8(buf, dataLength, data.data(),
+ dataLength);
+ EXPECT_EQ(rc, 0);
+
+ ASSERT_EQ(pldm_msgbuf_complete_consumed(buf), 0);
+
+ uint32_t respDataIntegrityChecksum = 0;
+
+ rc = decode_pldm_base_multipart_receive_resp(
+ responseMsg, payload_length, &resp_data, &respDataIntegrityChecksum);
+
+ /*
+ * pldm_msgbuf_extract_p() returns -EOVERFLOW as
+ * decode_pldm_base_multipart_receive_resp() tried to consume more than
+ * the expected input message buffer.
+ */
+ EXPECT_EQ(rc, -EOVERFLOW);
+}
+#endif
+
+#ifdef LIBPLDM_API_TESTING
TEST(EncodeMultipartReceiveResponse, GoodTestWithChecksum)
{
uint8_t instance_id = 0;