[BUG] Bug State Transition From FIN_WAIT_1.

[Fr3ya]

Describe the bug
The current implementation of the TCP state machine in FreeRTOS_TCP_State_Handling.c does not fully follow RFC793 behavior for connection termination when handling simultaneous close ( both sides send FIN). The stack transitions to CLOSE_WAIT rather than properly handling the CLOSING/TIME_WAIT states as specified by the RFC793.

Specifically, as specified in RFC793 [Page 74], when the state is in FIN-WAIT-1 STATE, it specifies:

If our FIN has been ACKed (perhaps in this segment), then enter TIME-WAIT, start the time-wait timer, turn off the other timers; otherwise, enter the CLOSING state.

When is FIN-WAIT2

Enter the TIME-WAIT state.  Start the time-wait timer, turn off the other timers.

However, In FreeRTOS_TCP_State_Handling.c, when the state is eFIN_WAIT_1 or eFIN_WAIT_2, it goes into the function prvTCPHandleFin. However, the implementation, when FIN is ACKed, it enters to CLOSE_WAIT (Line 302) instead of TIME_WAIT. When not ACKed, the implementation enters into LAST_ACK (Line 273) instead of CLOSING specified.

FreeRTOS-Plus-TCP/source/FreeRTOS_TCP_State_Handling.c

Lines 231 to 302 in 22c095e

	static BaseType_t prvTCPHandleFin( FreeRTOS_Socket_t * pxSocket,
	const NetworkBufferDescriptor_t * pxNetworkBuffer )
	{
	/* Map the ethernet buffer onto the ProtocolHeader_t struct for easy access to the fields. */
	/* MISRA Ref 11.3.1 [Misaligned access] */
	/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
	/* coverity[misra_c_2012_rule_11_3_violation] */
	ProtocolHeaders_t * pxProtocolHeaders = ( ( ProtocolHeaders_t * )
	&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) );
	TCPHeader_t * pxTCPHeader = &( pxProtocolHeaders->xTCPHeader );
	uint8_t ucIntermediateResult = 0, ucTCPFlags = pxTCPHeader->ucTCPFlags;
	TCPWindow_t * pxTCPWindow = &pxSocket->u.xTCP.xTCPWindow;
	BaseType_t xSendLength = 0;
	uint32_t ulAckNr = FreeRTOS_ntohl( pxTCPHeader->ulAckNr );
	if( ( ucTCPFlags & tcpTCP_FLAG_FIN ) != 0U )
	{
	pxTCPWindow->rx.ulCurrentSequenceNumber = pxTCPWindow->rx.ulFINSequenceNumber + 1U;
	}
	if( pxSocket->u.xTCP.bits.bFinSent == pdFALSE_UNSIGNED )
	{
	/* We haven't yet replied with a FIN, do so now. */
	pxTCPWindow->tx.ulFINSequenceNumber = pxTCPWindow->tx.ulCurrentSequenceNumber;
	pxSocket->u.xTCP.bits.bFinSent = pdTRUE_UNSIGNED;
	}
	else
	{
	/* We did send a FIN already, see if it's ACK'd. */
	if( ulAckNr == ( pxTCPWindow->tx.ulFINSequenceNumber + 1U ) )
	{
	pxSocket->u.xTCP.bits.bFinAcked = pdTRUE_UNSIGNED;
	}
	}
	if( pxSocket->u.xTCP.bits.bFinAcked == pdFALSE_UNSIGNED )
	{
	pxTCPWindow->tx.ulCurrentSequenceNumber = pxTCPWindow->tx.ulFINSequenceNumber;
	pxTCPHeader->ucTCPFlags = ( uint8_t ) tcpTCP_FLAG_ACK | ( uint8_t ) tcpTCP_FLAG_FIN;
	/* And wait for the final ACK. */
	vTCPStateChange( pxSocket, eLAST_ACK );
	}
	else
	{
	/* Our FIN has been ACK'd, the outgoing sequence number is now fixed. */
	pxTCPWindow->tx.ulCurrentSequenceNumber = pxTCPWindow->tx.ulFINSequenceNumber + 1U;
	if( pxSocket->u.xTCP.bits.bFinRecv == pdFALSE_UNSIGNED )
	{
	/* We have sent out a FIN but the peer hasn't replied with a FIN
	* yet. Do nothing for the moment. */
	pxTCPHeader->ucTCPFlags = 0U;
	}
	else
	{
	if( pxSocket->u.xTCP.bits.bFinLast == pdFALSE_UNSIGNED )
	{
	/* This is the third of the three-way hand shake: the last
	* ACK. */
	pxTCPHeader->ucTCPFlags = tcpTCP_FLAG_ACK;
	}
	else
	{
	/* The other party started the closure, so we just wait for the
	* last ACK. */
	pxTCPHeader->ucTCPFlags = 0U;
	}
	/* And wait for the user to close this socket. */
	vTCPStateChange( pxSocket, eCLOSE_WAIT );

[htibosch]

Hi @Fr3ya , thank you for reporting this.
I'm sorry to say that at this moment we have no time/capacity to work on it. If I may believe the feedback, TCP stack works perfectly as it is. And if we change the logic, there is a possibility of breaking existing code.
I know there are some minor differences between FreeRTOS+TCP and "the standard", but it doesn't lead to erroneous behaviour.
We are currently concentrating on new LTS releases.

Thanks again,
Hein

[Fr3ya]

Hi @htibosch, thanks for your response. I also notice another point:

The TCP state machine implementation takes a simplified approach when handling connection termination: it bypasses the CLOSE_WAIT state and transitions directly from ESTABLISHED to LAST_ACK when receiving a FIN from the peer (under certain buffer conditions). This behavior differs from RFC 793 Section 3.5, which specifies that the CLOSE_WAIT state should be entered to allow the application to explicitly close the connection.

I'd like to understand whether this is an intentional design decision for embedded systems optimization, or if it's an oversight Thanks

Code Location
File: source/FreeRTOS_TCP_State_Handling.c

prvHandleEstablished() (lines 582-641): When FIN is received, checks buffer conditions and calls prvTCPHandleFin()
// Lines 592-594: Automatic close decision based on buffer state
bRxComplete = xTCPWindowRxEmpty( pxTCPWindow );
bTxDone = xTCPWindowTxDone( pxTCPWindow );

if( ( bRxComplete == 0 ) || ( bTxDone == 0 ) )
{
    xMayClose = pdFALSE;  // Only refuse if buffers not empty
}

prvTCPHandleFin() (lines 267-273): Transitions directly to eLAST_ACK
if( pxSocket->u.xTCP.bits.bFinAcked == pdFALSE_UNSIGNED )
{
    pxTCPWindow->tx.ulCurrentSequenceNumber = pxTCPWindow->tx.ulFINSequenceNumber;
    pxTCPHeader->ucTCPFlags = ( uint8_t ) tcpTCP_FLAG_ACK | ( uint8_t ) tcpTCP_FLAG_FIN;

    /* And wait for the final ACK. */
    vTCPStateChange( pxSocket, eLAST_ACK );
}