Skip to content

Commit

Permalink
[ Cherry pick from master ] 2 TCP Bug fixes (#37327)
Browse files Browse the repository at this point in the history
* TCP tests: TC-SC-8.x - Use ArmFailsafe as cmd (#37313)

* TCP tests: TC-SC-8.x - Use ArmFailsafe as cmd

Also add top-level pics

* Fix payload capability

* Fix for Bug #36732 (#36879)

Set the app_state callback object in the Connection state to null
when the CASE session object is being cleared, on top of setting the
inner callback methods to null.
This prevents the callback object from being accessed later, when the
connection is getting closed(after the CASE session has been set up and
the session object no longer exists).

* Fix for Bug #36731. (#36880)

Add CloseActiveConnections() call in TCPBase::Close(), which
is called as part of Server::Shutdown().
Active connections should be closed as part of Server shutdown.
This allows the TCPConnectionState to also close the associated
TCPEndpoint object as part of this shutdown flow.

Previously, the CloseActiveConnections() call was present in the
TCPBase destructor alone.

Add test for Connection Close() and checking for TCPEndPoint.

---------

Co-authored-by: C Freeman <[email protected]>
  • Loading branch information
pidarped and cecille authored Jan 31, 2025
1 parent fc00c97 commit 3338248
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 14 deletions.
18 changes: 13 additions & 5 deletions src/protocols/secure_channel/CASESession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -428,12 +428,20 @@ void CASESession::Clear()
mTCPConnCbCtxt.connClosedCb = nullptr;
mTCPConnCbCtxt.connReceivedCb = nullptr;

if (mPeerConnState && mPeerConnState->mConnectionState != Transport::TCPState::kConnected)
if (mPeerConnState)
{
// Abort the connection if the CASESession is being destroyed and the
// connection is in the middle of being set up.
mSessionManager->TCPDisconnect(mPeerConnState, /* shouldAbort = */ true);
mPeerConnState = nullptr;
// Set the app state callback object in the Connection state to null
// to prevent any dangling pointer to memory(mTCPConnCbCtxt) owned
// by the CASESession object, that is now getting cleared.
mPeerConnState->mAppState = nullptr;

if (mPeerConnState->mConnectionState != Transport::TCPState::kConnected)
{
// Abort the connection if the CASESession is being destroyed and the
// connection is in the middle of being set up.
mSessionManager->TCPDisconnect(mPeerConnState, /* shouldAbort = */ true);
mPeerConnState = nullptr;
}
}
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
}
Expand Down
5 changes: 4 additions & 1 deletion src/transport/raw/ActiveTCPConnectionState.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ struct ActiveTCPConnectionState

void Free()
{
mEndPoint->Free();
if (mEndPoint)
{
mEndPoint->Free();
}
mPeerAddr = PeerAddress::Uninitialized();
mEndPoint = nullptr;
mReceived = nullptr;
Expand Down
13 changes: 5 additions & 8 deletions src/transport/raw/TCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,8 @@ constexpr int kListenBacklogSize = 2;

TCPBase::~TCPBase()
{
if (mListenSocket != nullptr)
{
// endpoint is only non null if it is initialized and listening
mListenSocket->Free();
mListenSocket = nullptr;
}

CloseActiveConnections();
// Call Close to free the listening socket and close all active connections.
Close();
}

void TCPBase::CloseActiveConnections()
Expand Down Expand Up @@ -125,6 +119,9 @@ void TCPBase::Close()
mListenSocket->Free();
mListenSocket = nullptr;
}

CloseActiveConnections();

mState = TCPState::kNotReady;
}

Expand Down
23 changes: 23 additions & 0 deletions src/transport/raw/tests/TestTCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,29 @@ TEST_F(TestTCP, HandleConnCloseCalledTest6)
HandleConnCloseTest(addr);
}

TEST_F(TestTCP, CheckTCPEndpointAfterCloseTest)
{
TCPImpl tcp;

IPAddress addr;
IPAddress::FromString("::1", addr);

MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext);
gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr);
gMockTransportMgrDelegate.ConnectTest(tcp, addr);

Transport::PeerAddress lPeerAddress = Transport::PeerAddress::TCP(addr, gChipTCPPort);
void * state = TestAccess::FindActiveConnection(tcp, lPeerAddress);
ASSERT_NE(state, nullptr);
TCPEndPoint * lEndPoint = TestAccess::GetEndpoint(state);
ASSERT_NE(lEndPoint, nullptr);

// Call Close and check the TCPEndpoint
tcp.Close();
lEndPoint = TestAccess::GetEndpoint(state);
ASSERT_EQ(lEndPoint, nullptr);
}

TEST_F(TestTCP, CheckProcessReceivedBuffer)
{
TCPImpl tcp;
Expand Down

0 comments on commit 3338248

Please sign in to comment.