Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore DeviceCommissioner::OnDeviceConnectionFailureFn if it's for the wrong node #22244

Closed
bzbarsky-apple opened this issue Aug 29, 2022 · 0 comments · Fixed by #22247
Closed
Assignees
Labels

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Aug 29, 2022

Problem

DeviceCommissioner::OnDeviceConnectedFn is ignored if it's not for mDeviceBeingCommissioned. We should do the same with DeviceCommissioner::OnDeviceConnectionFailureFn, because otherwise we might end up calling into CommissioningStageComplete without a valid mDeviceBeingCommissioned and crash with a stack like:

0   chip::Controller::DeviceCommissioner::CommissioningStageComplete(chip::ChipError, chip::Controller::CommissioningDelegate::CommissioningReport) + 88 (src/controller/CHIPDeviceController.cpp:1589)
1   chip::Controller::DeviceCommissioner::OnDeviceConnectionFailureFn(void*, chip::ScopedNodeId const&, chip::ChipError) + 372 (src/controller/CHIPDeviceController.cpp:1665)
2   chip::Controller::DeviceCommissioner::OnDeviceConnectionFailureFn(void*, chip::ScopedNodeId const&, chip::ChipError) + 372 (src/controller/CHIPDeviceController.cpp:1665)
3   chip::OperationalSessionSetup::DequeueConnectionCallbacks(chip::ChipError) + 408 (src/app/OperationalSessionSetup.cpp:283)
4   chip::OperationalSessionSetup::OnSessionEstablishmentError(chip::ChipError) + 144 (src/app/OperationalSessionSetup.cpp:315)
5   chip::CASESession::AbortPendingEstablish(chip::ChipError) + 108 (src/protocols/secure_channel/CASESession.cpp:297)
6   chip::CASESession::OnResponseTimeout(chip::Messaging::ExchangeContext*) + 236 (src/protocols/secure_channel/CASESession.cpp:290)
7   chip::Messaging::ExchangeContext::NotifyResponseTimeout(bool) + 284 (src/messaging/ExchangeContext.cpp:481)

(on SHA 2e10854 for purposes of line numbers). This is one of the top crashes we are observing in our Matter testing.

which is crashing on the mDeviceBeingCommissioned dereference in DeviceCommissioner::CommissioningStageComplete

Proposed Solution

Add the relevant checks.

@bzbarsky-apple bzbarsky-apple self-assigned this Aug 29, 2022
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 29, 2022
… devices.

Just like we ignore DeviceCommissioner::OnDeviceConnectedFn if the
device id does not match mDeviceBeingCommissioned, we should ignore
OnDeviceConnectionFailureFn.

Fixes project-chip#22244
andy31415 pushed a commit that referenced this issue Aug 30, 2022
… devices. (#22247)

Just like we ignore DeviceCommissioner::OnDeviceConnectedFn if the
device id does not match mDeviceBeingCommissioned, we should ignore
OnDeviceConnectionFailureFn.

Fixes #22244
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this issue Sep 16, 2022
… devices. (project-chip#22247)

Just like we ignore DeviceCommissioner::OnDeviceConnectedFn if the
device id does not match mDeviceBeingCommissioned, we should ignore
OnDeviceConnectionFailureFn.

Fixes project-chip#22244
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant