No longer provide OperationDeviceProxy in OnDeviceConnected callback (#21256)

Previous implementations of OnDeviceConnected held onto OperationalDeviceProxy when they really should not have could lead to use after free should something else free that OperationalDeviceProxy.

Author: tehampson

examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp

@@ -67,7 +67,8 @@ static void RegisterSwitchCommands()
 }
 #endif // defined(ENABLE_CHIP_SHELL)
 
-static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, chip::DeviceProxy * peer_device, void * context)
+static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, chip::OperationalDeviceProxy * peer_device,
+                                      void * context)
 {
     using namespace chip;
     using namespace chip::app;
@@ -88,6 +89,7 @@ static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, ch
             ChipLogError(NotSpecified, "OnOff command failed: %" CHIP_ERROR_FORMAT, error.Format());
         };
 
+        VerifyOrDie(peer_device != nullptr && peer_device->ConnectionReady());
         if (sSwitchOnOffState)
         {
             Clusters::OnOff::Commands::On::Type onCommand;

examples/all-clusters-app/ameba/main/BindingHandler.cpp

@@ -50,7 +50,8 @@ Engine sShellSwitchBindingSubCommands;
 
 namespace {
 
-void ProcessOnOffUnicastBindingCommand(CommandId commandId, const EmberBindingTableEntry & binding, DeviceProxy * peer_device)
+void ProcessOnOffUnicastBindingCommand(CommandId commandId, const EmberBindingTableEntry & binding,
+                                       OperationalDeviceProxy * peer_device)
 {
     auto onSuccess = [](const ConcreteCommandPath & commandPath, const StatusIB & status, const auto & dataResponse) {
         ChipLogProgress(NotSpecified, "OnOff command succeeds");
@@ -60,6 +61,7 @@ void ProcessOnOffUnicastBindingCommand(CommandId commandId, const EmberBindingTa
         ChipLogError(NotSpecified, "OnOff command failed: %" CHIP_ERROR_FORMAT, error.Format());
     };
 
+    VerifyOrDie(peer_device != nullptr && peer_device->ConnectionReady());
     switch (commandId)
     {
     case Clusters::OnOff::Commands::Toggle::Id:
@@ -106,7 +108,7 @@ void ProcessOnOffGroupBindingCommand(CommandId commandId, const EmberBindingTabl
     }
 }
 
-void LightSwitchChangedHandler(const EmberBindingTableEntry & binding, DeviceProxy * peer_device, void * context)
+void LightSwitchChangedHandler(const EmberBindingTableEntry & binding, OperationalDeviceProxy * peer_device, void * context)
 {
     VerifyOrReturn(context != nullptr, ChipLogError(NotSpecified, "OnDeviceConnectedFn: context is null"));
     BindingCommandData * data = static_cast<BindingCommandData *>(context);

examples/all-clusters-app/esp32/main/include/ShellCommands.h

@@ -134,7 +134,7 @@ class CASECommands
 
 private:
     CASECommands() {}
-    static void OnConnected(void * context, OperationalDeviceProxy * deviceProxy)
+    static void OnConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
     {
         streamer_printf(streamer_get(), "Establish CASESession Success!\r\n");
         GetInstance().SetOnConnecting(false);

examples/all-clusters-app/nxp/mw320/binding-handler.cpp

@@ -68,7 +68,8 @@ static void RegisterSwitchCommands()
 }
 #endif // defined(ENABLE_CHIP_SHELL)
 
-static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, chip::DeviceProxy * peer_device, void * context)
+static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, chip::OperationalDeviceProxy * peer_device,
+                                      void * context)
 {
     using namespace chip;
     using namespace chip::app;
@@ -92,6 +93,7 @@ static void BoundDeviceChangedHandler(const EmberBindingTableEntry & binding, ch
         // command (SwitchCommandHandler)
         {
             Clusters::OnOff::Commands::Toggle::Type toggleCommand;
+            VerifyOrDie(peer_device != nullptr && peer_device->ConnectionReady());
             Controller::InvokeCommandRequest(peer_device->GetExchangeManager(), peer_device->GetSecureSession().Value(),
                                              binding.remote, toggleCommand, onSuccess, onFailure);
         }

examples/all-clusters-minimal-app/esp32/main/include/ShellCommands.h

@@ -134,7 +134,7 @@ class CASECommands
 
 private:
     CASECommands() {}
-    static void OnConnected(void * context, OperationalDeviceProxy * deviceProxy)
+    static void OnConnected(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle)
     {
         streamer_printf(streamer_get(), "Establish CASESession Success!\r\n");
         GetInstance().SetOnConnecting(false);

examples/chip-tool/commands/clusters/ModelCommand.cpp

@@ -46,12 +46,14 @@ CHIP_ERROR ModelCommand::RunCommand()
                                                     &mOnDeviceConnectionFailureCallback);
 }
 
-void ModelCommand::OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device)
+void ModelCommand::OnDeviceConnectedFn(void * context, chip::Messaging::ExchangeManager & exchangeMgr,
+                                       chip::SessionHandle & sessionHandle)
 {
     ModelCommand * command = reinterpret_cast<ModelCommand *>(context);
     VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "OnDeviceConnectedFn: context is null"));
 
-    CHIP_ERROR err = command->SendCommand(device, command->mEndPointId);
+    chip::OperationalDeviceProxy device(&exchangeMgr, sessionHandle);
+    CHIP_ERROR err = command->SendCommand(&device, command->mEndPointId);
     VerifyOrReturn(CHIP_NO_ERROR == err, command->SetCommandExitStatus(err));
 }
 

examples/chip-tool/commands/clusters/ModelCommand.h

@@ -69,7 +69,8 @@ class ModelCommand : public CHIPCommand
     chip::NodeId mDestinationId;
     std::vector<chip::EndpointId> mEndPointId;
 
-    static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
+    static void OnDeviceConnectedFn(void * context, chip::Messaging::ExchangeManager & exchangeMgr,
+                                    chip::SessionHandle & sessionHandle);
     static void OnDeviceConnectionFailureFn(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error);
 
     chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;

examples/chip-tool/commands/pairing/CloseSessionCommand.cpp

@@ -28,17 +28,16 @@ CHIP_ERROR CloseSessionCommand::RunCommand()
     CommissioneeDeviceProxy * commissioneeDeviceProxy = nullptr;
     if (CHIP_NO_ERROR == CurrentCommissioner().GetDeviceBeingCommissioned(mDestinationId, &commissioneeDeviceProxy))
     {
-        return CloseSession(commissioneeDeviceProxy);
+        VerifyOrReturnError(commissioneeDeviceProxy->GetSecureSession().HasValue(), CHIP_ERROR_INCORRECT_STATE);
+        return CloseSession(*commissioneeDeviceProxy->GetExchangeManager(), commissioneeDeviceProxy->GetSecureSession().Value());
     }
 
     return CurrentCommissioner().GetConnectedDevice(mDestinationId, &mOnDeviceConnectedCallback,
                                                     &mOnDeviceConnectionFailureCallback);
 }
 
-CHIP_ERROR CloseSessionCommand::CloseSession(DeviceProxy * device)
+CHIP_ERROR CloseSessionCommand::CloseSession(Messaging::ExchangeManager & exchangeMgr, const SessionHandle & sessionHandle)
 {
-    VerifyOrReturnError(device->GetSecureSession().HasValue(), CHIP_ERROR_INCORRECT_STATE);
-
     // TODO perhaps factor out this code into something on StatusReport that
     // takes an exchange and maybe a SendMessageFlags?
     SecureChannel::StatusReport statusReport(SecureChannel::GeneralStatusCode::kSuccess, SecureChannel::Id,
@@ -51,7 +50,7 @@ CHIP_ERROR CloseSessionCommand::CloseSession(DeviceProxy * device)
     System::PacketBufferHandle msg = bbuf.Finalize();
     VerifyOrReturnError(!msg.IsNull(), CHIP_ERROR_NO_MEMORY);
 
-    auto * exchange = device->GetExchangeManager()->NewContext(device->GetSecureSession().Value(), nullptr);
+    auto * exchange = exchangeMgr.NewContext(sessionHandle, nullptr);
     VerifyOrReturnError(exchange != nullptr, CHIP_ERROR_NO_MEMORY);
 
     // Per spec, CloseSession reports are always sent with MRP disabled.
@@ -69,12 +68,13 @@ CHIP_ERROR CloseSessionCommand::CloseSession(DeviceProxy * device)
     return err;
 }
 
-void CloseSessionCommand::OnDeviceConnectedFn(void * context, OperationalDeviceProxy * device)
+void CloseSessionCommand::OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr,
+                                              SessionHandle & sessionHandle)
 {
     auto * command = reinterpret_cast<CloseSessionCommand *>(context);
     VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "OnDeviceConnectedFn: context is null"));
 
-    CHIP_ERROR err = command->CloseSession(device);
+    CHIP_ERROR err = command->CloseSession(exchangeMgr, sessionHandle);
     VerifyOrReturn(CHIP_NO_ERROR == err, command->SetCommandExitStatus(err));
 }
 

examples/chip-tool/commands/pairing/CloseSessionCommand.h

@@ -19,7 +19,7 @@
 #pragma once
 
 #include "../common/CHIPCommand.h"
-#include <app/OperationalDeviceProxy.h>
+#include <app/OperationalSessionSetup.h>
 #include <lib/core/CHIPCallback.h>
 #include <lib/core/DataModelTypes.h>
 
@@ -46,11 +46,12 @@ class CloseSessionCommand : public CHIPCommand
     chip::NodeId mDestinationId;
     chip::Optional<uint16_t> mTimeoutSecs;
 
-    static void OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device);
+    static void OnDeviceConnectedFn(void * context, chip::Messaging::ExchangeManager & exchangeMgr,
+                                    chip::SessionHandle & sessionHandle);
     static void OnDeviceConnectionFailureFn(void * context, const chip::ScopedNodeId & peerId, CHIP_ERROR error);
 
     // Try to send the action CloseSession status report.
-    CHIP_ERROR CloseSession(chip::DeviceProxy * device);
+    CHIP_ERROR CloseSession(chip::Messaging::ExchangeManager & exchangeMgr, const chip::SessionHandle & sessionHandle);
 
     chip::Callback::Callback<chip::OnDeviceConnected> mOnDeviceConnectedCallback;
     chip::Callback::Callback<chip::OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -51,12 +51,13 @@ CHIP_ERROR TestCommand::WaitForCommissionee(const char * identity,
                                                         &mOnDeviceConnectionFailureCallback);
 }
 
-void TestCommand::OnDeviceConnectedFn(void * context, chip::OperationalDeviceProxy * device)
+void TestCommand::OnDeviceConnectedFn(void * context, chip::Messaging::ExchangeManager & exchangeMgr,
+                                      chip::SessionHandle & sessionHandle)
 {
     ChipLogProgress(chipTool, " **** Test Setup: Device Connected\n");
     auto * command = static_cast<TestCommand *>(context);
     VerifyOrReturn(command != nullptr, ChipLogError(chipTool, "Device connected, but cannot run the test, as the context is null"));
-    command->mDevices[command->GetIdentity()] = device;
+    command->mDevices[command->GetIdentity()] = std::make_unique<chip::OperationalDeviceProxy>(&exchangeMgr, sessionHandle);
 
     LogErrorOnFailure(command->ContinueOnChipMainThread(CHIP_NO_ERROR));
 }