From 25217842574ecf2f6f556449cce386fd606c826b Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Wed, 16 Mar 2022 15:54:33 +0100 Subject: [PATCH] Fix segfault caused by accessing released device object (#16168) * Fix segfault caused by accessing released device object Local reference to the device being commissioned has to be cleared when the device object is released. Otherwise, we will have a local pointer to freed memory. * Send operational certificate to given device Given device proxy object might not necessarily be a device currently being commissioned. --- src/controller/CHIPDeviceController.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 8fc7096f35658a..161d56d3dd4b28 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -745,6 +745,11 @@ CommissioneeDeviceProxy * DeviceCommissioner::FindCommissioneeDevice(NodeId id) void DeviceCommissioner::ReleaseCommissioneeDevice(CommissioneeDeviceProxy * device) { mCommissioneeDevicePool.ReleaseObject(device); + // Make sure that there will be no dangling pointer + if (mDeviceBeingCommissioned == device) + { + mDeviceBeingCommissioned = nullptr; + } } CHIP_ERROR DeviceCommissioner::GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** out_device) @@ -887,7 +892,6 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re if (device != nullptr) { ReleaseCommissioneeDevice(device); - mDeviceBeingCommissioned = nullptr; } } @@ -972,7 +976,6 @@ void DeviceCommissioner::RendezvousCleanup(CHIP_ERROR status) // for IP commissioning, we have taken a reference to the // operational node to send the completion command. ReleaseCommissioneeDevice(mDeviceBeingCommissioned); - mDeviceBeingCommissioned = nullptr; } if (mPairingDelegate != nullptr) @@ -1243,8 +1246,8 @@ CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device, request.caseAdminNode = adminSubject; request.adminVendorId = mVendorId; - ReturnErrorOnFailure(SendCommand(mDeviceBeingCommissioned, request, - OnOperationalCertificateAddResponse, OnAddNOCFailureResponse)); + ReturnErrorOnFailure( + SendCommand(device, request, OnOperationalCertificateAddResponse, OnAddNOCFailureResponse)); ChipLogProgress(Controller, "Sent operational certificate to the device"); @@ -1466,7 +1469,7 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin { // Commissioning delegate will only return error if it failed to perform the appropriate commissioning step. // In this case, we should call back the commissioning complete and call session error - if (mPairingDelegate != nullptr) + if (mPairingDelegate != nullptr && mDeviceBeingCommissioned != nullptr) { mPairingDelegate->OnCommissioningComplete(mDeviceBeingCommissioned->GetDeviceId(), status); } @@ -1487,7 +1490,6 @@ void DeviceCommissioner::OnDeviceConnectedFn(void * context, OperationalDevicePr // Let's release the device that's being paired, if pairing was successful, // and the device is available on the operational network. commissioner->ReleaseCommissioneeDevice(commissioner->mDeviceBeingCommissioned); - commissioner->mDeviceBeingCommissioned = nullptr; if (commissioner->mCommissioningDelegate != nullptr) { CommissioningDelegate::CommissioningReport report; @@ -1544,7 +1546,6 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, PeerId peer // // Run the above cases under valgrind/asan to validate no additional leaks. commissioner->ReleaseCommissioneeDevice(commissioner->mDeviceBeingCommissioned); - commissioner->mDeviceBeingCommissioned = nullptr; } }