From f6e0ff4cbcdd02752599ef0d253e5cee887e3721 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 29 Aug 2022 16:53:40 -0400 Subject: [PATCH] Fix crashes when DeviceCommissioner is shut down. There are two fixes here: 1) Ensure that DeviceCommissioner::Shutdown properly cleans up mDeviceBeingCommissioned so we don't end up with it being a dangling pointer. 2) Ensure that DeviceCommissioner::CommissioningStageComplete doesn't try to derefence a null mDeviceBeingCommissioned (which indicates something has already stopped the commissioning process). Fixes https://github.com/project-chip/connectedhomeip/issues/22243 --- src/controller/CHIPDeviceController.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 8bcba1f5c06564..88bef7820efd71 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -489,6 +489,17 @@ void DeviceCommissioner::Shutdown() } #endif // CHIP_DEVICE_CONFIG_ENABLE_COMMISSIONER_DISCOVERY + // If we have a commissionee device for the device being commissioned, + // release it now, before we release our whole commissionee pool. + if (mDeviceBeingCommissioned != nullptr) + { + auto * commissionee = FindCommissioneeDevice(mDeviceBeingCommissioned->GetDeviceId()); + if (commissionee) + { + ReleaseCommissioneeDevice(commissionee); + } + } + // Release everything from the commissionee device pool here. DeviceController::Shutdown releases operational. mCommissioneeDevicePool.ReleaseAll(); @@ -1561,6 +1572,15 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin { // Once this stage is complete, reset mDeviceBeingCommissioned - this will be reset when the delegate calls the next step. MATTER_TRACE_EVENT_SCOPE("CommissioningStageComplete", "DeviceCommissioner"); + + if (mDeviceBeingCommissioned == nullptr) + { + // We are getting a stray callback (e.g. due to un-cancellable + // operations) when we are not in fact commissioning anything. Just + // ignore it. + return; + } + NodeId nodeId = mDeviceBeingCommissioned->GetDeviceId(); DeviceProxy * proxy = mDeviceBeingCommissioned; mDeviceBeingCommissioned = nullptr;