commit 3fcfaa1228bbb1bf3f92286f831b589dfbd0e06a
author Manojkiran Eda <manojkiran.eda@gmail.com> Mon Feb 26 15:17:58 2024 +0530
committer ManojKiran Eda <manojkiran.eda@gmail.com> Wed Feb 28 12:32:17 2024 +0000
tree 60ae79a00a4d1f82124c2004dd968a80a2d13787
parent 5ef5b340129b2f8eb1f4afc3c4780a7bd739cbd8

Fix softoff crash due to double free corruption

In its present condition, the instance database gets initialized
within the hostSoftOff function. Consequently, once the function
execution is complete, the database goes out of scope, triggering
the invocation of the instanceDb destructor. This, in turn,
prematurely destroys the instance database in the middle of the
softpoweroff flow, resulting in a double free corruption.

Tested:
After implementing the fix, softoff was tested to ensure it does
not crash.

Change-Id: I251201b06864a8a5273bfaa23468a06ad678763a
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>

softoff/main.cpp

diff --git a/softoff/main.cpp b/softoff/main.cpp
index e5f61d2..cfbbcd2 100644
--- a/softoff/main.cpp
+++ b/softoff/main.cpp
@@ -1,3 +1,4 @@
+#include "common/instance_id.hpp"
 #include "common/utils.hpp"
 #include "softoff.hpp"
 
@@ -13,10 +14,13 @@
     // Get a handle to system D-Bus.
     auto& bus = pldm::utils::DBusHandler::getBus();
 
+    // Obtain the instance database
+    pldm::InstanceIdDb instanceIdDb;
+
     // Attach the bus to sd_event to service user requests
     bus.attach_event(event.get(), SD_EVENT_PRIORITY_NORMAL);
 
-    pldm::SoftPowerOff softPower(bus, event.get());
+    pldm::SoftPowerOff softPower(bus, event.get(), instanceIdDb);
 
     if (softPower.isError())
     {

softoff/softoff.cpp

diff --git a/softoff/softoff.cpp b/softoff/softoff.cpp
index 78a59ed..6f24991 100644
--- a/softoff/softoff.cpp
+++ b/softoff/softoff.cpp
@@ -30,8 +30,10 @@
 constexpr pldm::pdr::TerminusID TID = 0; // TID will be implemented later.
 namespace sdbusRule = sdbusplus::bus::match::rules;
 
-SoftPowerOff::SoftPowerOff(sdbusplus::bus_t& bus, sd_event* event) :
-    bus(bus), timer(event)
+SoftPowerOff::SoftPowerOff(sdbusplus::bus_t& bus, sd_event* event,
+                           pldm::InstanceIdDb& instanceIdDb) :
+    bus(bus),
+    timer(event), instanceIdDb(instanceIdDb)
 {
     getHostState();
     if (hasError || completed)
@@ -306,7 +308,6 @@
     auto request = reinterpret_cast<pldm_msg*>(requestMsg.data());
     set_effecter_state_field stateField{
         PLDM_REQUEST_SET, PLDM_SW_TERM_GRACEFUL_SHUTDOWN_REQUESTED};
-    pldm::InstanceIdDb instanceIdDb;
     instanceID = instanceIdDb.next(pldmTID);
     auto rc = encode_set_state_effecter_states_req(
         instanceID, effecterID, effecterCount, &stateField, request);

softoff/softoff.hpp

diff --git a/softoff/softoff.hpp b/softoff/softoff.hpp
index 2bf3968..edec74e 100644
--- a/softoff/softoff.hpp
+++ b/softoff/softoff.hpp
@@ -1,5 +1,6 @@
 #pragma once
 
+#include "common/instance_id.hpp"
 #include "common/transport.hpp"
 #include "common/types.hpp"
 
@@ -22,8 +23,10 @@
      *
      *  @param[in] bus       - system D-Bus handler
      *  @param[in] event     - sd_event handler
+     *  @param[in] instanceDb - pldm instance database
      */
-    SoftPowerOff(sdbusplus::bus_t& bus, sd_event* event);
+    SoftPowerOff(sdbusplus::bus_t& bus, sd_event* event,
+                 InstanceIdDb& instanceIdDb);
 
     /** @brief Is the pldm-softpoweroff has error.
      * if hasError is true, that means the pldm-softpoweroff failed to
@@ -142,6 +145,10 @@
      * to BMC's pldmd, and the pldmd will emit the StateSensorEvent signal.
      **/
     std::unique_ptr<sdbusplus::bus::match_t> pldmEventSignal;
+
+    /** @brief Reference to the instance database
+     */
+    InstanceIdDb& instanceIdDb;
 };
 
 } // namespace pldm