Skip to content

Commit

Permalink
Fix segfault caused by accessing released device object (#16168)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
arkq authored and pull[bot] committed Jan 5, 2024
1 parent 8729f1b commit 2521784
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -887,7 +892,6 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
if (device != nullptr)
{
ReleaseCommissioneeDevice(device);
mDeviceBeingCommissioned = nullptr;
}
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1243,8 +1246,8 @@ CHIP_ERROR DeviceCommissioner::SendOperationalCertificate(DeviceProxy * device,
request.caseAdminNode = adminSubject;
request.adminVendorId = mVendorId;

ReturnErrorOnFailure(SendCommand<OperationalCredentialsCluster>(mDeviceBeingCommissioned, request,
OnOperationalCertificateAddResponse, OnAddNOCFailureResponse));
ReturnErrorOnFailure(
SendCommand<OperationalCredentialsCluster>(device, request, OnOperationalCertificateAddResponse, OnAddNOCFailureResponse));

ChipLogProgress(Controller, "Sent operational certificate to the device");

Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
}

Expand Down

0 comments on commit 2521784

Please sign in to comment.