commit f045e837f375378221f606e8ea0b124909452137
author Matt Spinler <spinler@us.ibm.com> Thu Mar 13 11:07:10 2025 -0500
committer Matt Spinler <spinler@us.ibm.com> Thu Mar 13 11:23:30 2025 -0500
tree ce9227f831aeb291b49a00d0bb1ba1299ed22c0e
parent d903057338a9bac97d06c9d8f6bbebc5b7478179

PEL: Ensure PLDM response valid before using it

Make sure that pldm_transport_recv_msg() was successful before passing
the response value it returns into pldm_msg_hdr_correlate_response().
Otherwise, a null pointer may be passed into it which will cause a
crash.

Also, instead of manually freeing the response on the function exit
points, just wrap it in a small Response struct that will free it in its
destructor.

Change-Id: I213c44758c248d14d83144a7bad397e84fe5602b
Signed-off-by: Matt Spinler <spinler@us.ibm.com>

extensions/openpower-pels/pldm_interface.cpp

diff --git a/extensions/openpower-pels/pldm_interface.cpp b/extensions/openpower-pels/pldm_interface.cpp
index 2c11468..a74017f 100644
--- a/extensions/openpower-pels/pldm_interface.cpp
+++ b/extensions/openpower-pels/pldm_interface.cpp
@@ -261,6 +261,19 @@
     memcpy(&_requestHeader, request, sizeof(pldm_msg_hdr));
 }
 
+struct Response
+{
+    Response(void* r) : response(r) {}
+    ~Response()
+    {
+        if (response != nullptr)
+        {
+            free(response);
+        }
+    }
+    void* response = nullptr;
+};
+
 void PLDMInterface::receive(IO& /*io*/, int /*fd*/, uint32_t revents,
                             pldm_transport* transport)
 
@@ -278,13 +291,18 @@
     auto rc = pldm_transport_recv_msg(transport, &pldmTID, &responseMsg,
                                       &responseSize);
     struct pldm_msg_hdr* hdr = (struct pldm_msg_hdr*)responseMsg;
-    if ((pldmTID != _eid) ||
-        !pldm_msg_hdr_correlate_response(&_requestHeader, hdr))
+    Response r{responseMsg};
+
+    if (rc == PLDM_REQUESTER_SUCCESS)
     {
-        // We got a response to someone else's message. Ignore it.
-        return;
+        if ((pldmTID != _eid) ||
+            !pldm_msg_hdr_correlate_response(&_requestHeader, hdr))
+        {
+            // We got a response to someone else's message. Ignore it.
+            return;
+        }
     }
-    if (rc)
+    else
     {
         if (rc == PLDM_REQUESTER_NOT_RESP_MSG)
         {
@@ -299,12 +317,9 @@
                    static_cast<std::underlying_type_t<pldm_requester_rc_t>>(rc),
                    "ERRNO", e);
         status = ResponseStatus::failure;
-
-        responseMsg = nullptr;
     }
     if (hdr && (hdr->request || hdr->datagram))
     {
-        free(responseMsg);
         return;
     }
 
@@ -338,11 +353,6 @@
     }
 
     callResponseFunc(status);
-
-    if (responseMsg)
-    {
-        free(responseMsg);
-    }
 }
 
 void PLDMInterface::receiveTimerExpired()