From 3a0aaf19bd92c3f337fcea2cc09cd667659247c9 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 21 Jun 2022 11:33:13 -0400 Subject: [PATCH] Updates to fix fallout from #19502. We need to allow messages on inactive sessions to reach the exchange manager, because such sessions need to be able to deliver an MRP ack to an exchange waiting for one. We also don't want to crash on an attempt to transition from Inactive to Defunct state; the transition should just be ignored. This way if we start trying to transitionin to Defunct on MRP delivery failures we will not start crashing if such a failure happens on an Inactive session. --- src/messaging/ExchangeMgr.cpp | 8 +++++++- src/transport/SecureSession.cpp | 11 +++++++++-- src/transport/SecureSession.h | 1 + src/transport/SessionManager.cpp | 2 +- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/messaging/ExchangeMgr.cpp b/src/messaging/ExchangeMgr.cpp index 0063aa5d22264a..a50672417ed7f2 100644 --- a/src/messaging/ExchangeMgr.cpp +++ b/src/messaging/ExchangeMgr.cpp @@ -236,7 +236,13 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const packetHeader.GetDestinationGroupId().Value()); } - // Do not handle unsolicited messages on a inactive session. + // Do not handle messages that don't match an existing exchange on a + // inactive session, since we should not be creating new exchanges there. + if (!session->IsActiveSession()) { + ChipLogProgress(ExchangeManager, "Dropping message on inactive session that does not match an existing exchange"); + return; + } + // If it's not a duplicate message, search for an unsolicited message handler if it is marked as being sent by an initiator. // Since we didn't find an existing exchange that matches the message, it must be an unsolicited message. However all // unsolicited messages must be marked as being from an initiator. diff --git a/src/transport/SecureSession.cpp b/src/transport/SecureSession.cpp index 7db899ffa7bf6d..106ce1a6cba5de 100644 --- a/src/transport/SecureSession.cpp +++ b/src/transport/SecureSession.cpp @@ -46,6 +46,10 @@ const char * SecureSession::StateToString(State state) const return "kPendingEviction"; break; + case State::kInactive: + return "kInactive"; + break; + default: return "???"; break; @@ -89,9 +93,12 @@ void SecureSession::MarkAsDefunct() case State::kInactive: // - // Once a session is marked Inactive, we CANNOT bring it back to either being active or defunct. + // Once a session is marked Inactive, we CANNOT bring it back to either + // being active or defunct. But consumers may not really know this + // session is already inactive. Just ignore the call and stay in + // kInactive state. // - FALLTHROUGH; + return; case State::kPendingEviction: // // Once a session is headed for eviction, we CANNOT bring it back to either being active or defunct. diff --git a/src/transport/SecureSession.h b/src/transport/SecureSession.h index 45c05d02e422e2..3868a0063cd28f 100644 --- a/src/transport/SecureSession.h +++ b/src/transport/SecureSession.h @@ -144,6 +144,7 @@ class SecureSession : public Session, public ReferenceCountedAsSecureSession(); - if (!secureSession->IsDefunct() && !secureSession->IsActiveSession()) + if (!secureSession->IsDefunct() && !secureSession->IsActiveSession() && !secureSession->IsInactive()) { ChipLogError(Inet, "Secure transport received message on a session in an invalid state (state = '%s')", secureSession->GetStateStr());