Skip to content

Commit cf8728a

Browse files
kstribrnAmznkstribrnAmzn
committed
Child socket cleanup on TCP window allocation failure
prvTCPSocketCopy() stores a pointer to a new child socket in the listening socket's u.xTCP.pxPeerSocket field, but when prvTCPCreateWindow() fails the IPv4 and IPv6 listen paths free that child through a direct vSocketClose() call that bypasses the vTCPRemoveTCPChild() cleanup. With this change, the reference is cleared before the socket is closed.
1 parent b86ad08 commit cf8728a

5 files changed

Lines changed: 205 additions & 34 deletions

File tree

‎History.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
Documentation and download available at https://www.FreeRTOS.org/
22

3+
Changes to be included in an upcoming release:
4+
+ Fix child socket cleanup with ipconfigTCP_HANG_PROTECTION is disabled (#1372)
5+
We thank Ghaith Abdulreda for their suggestion.
6+
37
Changes between FreeRTOS-plus-TCP V4.4.2 and V4.4.1 released October 16, 2026:
48
+ Add Router Advertisement validations for RFC 4861 section 6.1.2
59
(#1369). An RA is now accepted only when it arrives from a link-local

‎source/FreeRTOS_Sockets.c‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2316,16 +2316,23 @@ void * vSocketClose( FreeRTOS_Socket_t * pxSocket )
23162316
pxOtherSocket = ( ( FreeRTOS_Socket_t * ) listGET_LIST_ITEM_OWNER( pxIterator ) );
23172317

23182318
if( ( pxOtherSocket->u.xTCP.eTCPState == eTCP_LISTEN ) &&
2319-
( pxOtherSocket->usLocalPort == usLocalPort ) &&
2320-
( pxOtherSocket->u.xTCP.usChildCount != 0U ) )
2319+
( pxOtherSocket->usLocalPort == usLocalPort ) )
23212320
{
2322-
pxOtherSocket->u.xTCP.usChildCount--;
2323-
FreeRTOS_debug_printf( ( "Lost: Socket %u now has %u / %u child%s\n",
2324-
pxOtherSocket->usLocalPort,
2325-
pxOtherSocket->u.xTCP.usChildCount,
2326-
pxOtherSocket->u.xTCP.usBacklog,
2327-
( pxOtherSocket->u.xTCP.usChildCount == 1U ) ? "" : "ren" ) );
2328-
break;
2321+
if( pxOtherSocket->u.xTCP.pxPeerSocket == pxSocketToDelete )
2322+
{
2323+
pxOtherSocket->u.xTCP.pxPeerSocket = NULL;
2324+
}
2325+
2326+
if( pxOtherSocket->u.xTCP.usChildCount != 0U )
2327+
{
2328+
pxOtherSocket->u.xTCP.usChildCount--;
2329+
FreeRTOS_debug_printf( ( "Lost: Socket %u now has %u / %u child%s\n",
2330+
pxOtherSocket->usLocalPort,
2331+
pxOtherSocket->u.xTCP.usChildCount,
2332+
pxOtherSocket->u.xTCP.usBacklog,
2333+
( pxOtherSocket->u.xTCP.usChildCount == 1U ) ? "" : "ren" ) );
2334+
break;
2335+
}
23292336
}
23302337
}
23312338
}

‎source/FreeRTOS_TCP_State_Handling.c‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,37 +1012,20 @@
10121012
}
10131013
#else
10141014
{
1015-
/* A reference to the new socket may be stored and the socket is marked
1016-
* as 'passable'. */
1017-
10181015
/* When bPassAccept is true, this socket may be returned in a call to
10191016
* accept(). */
10201017
pxNewSocket->u.xTCP.bits.bPassAccept = ipTRUE_BOOL;
1021-
1022-
if( pxSocket->u.xTCP.pxPeerSocket == NULL )
1023-
{
1024-
pxSocket->u.xTCP.pxPeerSocket = pxNewSocket;
1025-
}
10261018
}
10271019
#endif /* if ( ipconfigTCP_HANG_PROTECTION == 1 ) */
10281020

1029-
pxSocket->u.xTCP.usChildCount++;
1030-
1031-
if( pxSocket->u.xTCP.pxPeerSocket == NULL )
1032-
{
1033-
pxSocket->u.xTCP.pxPeerSocket = pxNewSocket;
1034-
}
1035-
1036-
FreeRTOS_debug_printf( ( "Gain: Socket %u now has %u / %u child%s me: %p parent: %p peer: %p\n",
1037-
pxSocket->usLocalPort,
1038-
pxSocket->u.xTCP.usChildCount,
1039-
pxSocket->u.xTCP.usBacklog,
1040-
( pxSocket->u.xTCP.usChildCount == 1U ) ? "" : "ren",
1041-
( void * ) pxNewSocket,
1042-
( void * ) pxSocket,
1043-
pxSocket ? ( void * ) pxSocket->u.xTCP.pxPeerSocket : NULL ) );
1044-
1045-
/* Now bind the child socket to the same port as the listening socket. */
1021+
/* Now bind the child socket to the same port as the listening socket.
1022+
*
1023+
* The child is only counted and made reachable from the parent once the
1024+
* bind has succeeded. 'prvTCPSetSocketCount()' undoes both of those on
1025+
* every path which frees the child, and it locates the parent by the
1026+
* child's local port - which a child that failed to bind does not have.
1027+
* Linking an unbound child would therefore leave the parent holding a
1028+
* reference that no teardown path can clear. */
10461029
if( vSocketBind( pxNewSocket, &xAddress, sizeof( xAddress ), pdTRUE ) != 0 )
10471030
{
10481031
FreeRTOS_debug_printf( ( "TCP: Listen: new socket bind error\n" ) );
@@ -1051,6 +1034,24 @@
10511034
}
10521035
else
10531036
{
1037+
pxSocket->u.xTCP.usChildCount++;
1038+
1039+
/* Store a reference to the child in the listening socket, so that
1040+
* accept() can find it. */
1041+
if( pxSocket->u.xTCP.pxPeerSocket == NULL )
1042+
{
1043+
pxSocket->u.xTCP.pxPeerSocket = pxNewSocket;
1044+
}
1045+
1046+
FreeRTOS_debug_printf( ( "Gain: Socket %u now has %u / %u child%s me: %p parent: %p peer: %p\n",
1047+
pxSocket->usLocalPort,
1048+
pxSocket->u.xTCP.usChildCount,
1049+
pxSocket->u.xTCP.usBacklog,
1050+
( pxSocket->u.xTCP.usChildCount == 1U ) ? "" : "ren",
1051+
( void * ) pxNewSocket,
1052+
( void * ) pxSocket,
1053+
pxSocket ? ( void * ) pxSocket->u.xTCP.pxPeerSocket : NULL ) );
1054+
10541055
xResult = pdTRUE;
10551056
}
10561057

‎test/unit-test/FreeRTOS_Sockets/FreeRTOS_Sockets_privates_utest.c‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,108 @@ void test_prvTCPSetSocketCount_NotListeningSock_HappyPath( void )
14791479
TEST_ASSERT_EQUAL( 99, xChildSocket.u.xTCP.usChildCount );
14801480
}
14811481

1482+
/**
1483+
* @brief A child socket is being freed and the listening parent still refers to
1484+
* it. That reference must be dropped, or 'FreeRTOS_accept()' and
1485+
* 'FreeRTOS_select()' would dereference freed memory.
1486+
*/
1487+
void test_prvTCPSetSocketCount_NotListeningSock_ClearsPeerSocket( void )
1488+
{
1489+
FreeRTOS_Socket_t xSocketToDelete, xParentSocket;
1490+
ListItem_t xIterator;
1491+
uint16_t usLocalPort = 0x1234;
1492+
1493+
memset( &xSocketToDelete, 0, sizeof( xSocketToDelete ) );
1494+
memset( &xParentSocket, 0, sizeof( xParentSocket ) );
1495+
1496+
xSocketToDelete.ucProtocol = ( uint8_t ) FREERTOS_IPPROTO_TCP;
1497+
xSocketToDelete.u.xTCP.eTCPState = eCLOSED;
1498+
xSocketToDelete.usLocalPort = usLocalPort;
1499+
1500+
xParentSocket.u.xTCP.eTCPState = eTCP_LISTEN;
1501+
xParentSocket.usLocalPort = usLocalPort;
1502+
xParentSocket.u.xTCP.usChildCount = 1;
1503+
xParentSocket.u.xTCP.pxPeerSocket = &xSocketToDelete;
1504+
1505+
listGET_HEAD_ENTRY_ExpectAndReturn( ( List_t * ) &( xBoundTCPSocketsList ), ( ListItem_t * ) &( xIterator ) );
1506+
1507+
listGET_LIST_ITEM_OWNER_ExpectAndReturn( &xIterator, &xParentSocket );
1508+
1509+
prvTCPSetSocketCount( &xSocketToDelete );
1510+
1511+
TEST_ASSERT_EQUAL( NULL, xParentSocket.u.xTCP.pxPeerSocket );
1512+
TEST_ASSERT_EQUAL( 0, xParentSocket.u.xTCP.usChildCount );
1513+
}
1514+
1515+
/**
1516+
* @brief A child socket is being freed, but the listening parent refers to a
1517+
* different child which is still waiting to be accepted. That reference
1518+
* must survive: clearing it would lose a live connection.
1519+
*/
1520+
void test_prvTCPSetSocketCount_NotListeningSock_KeepsOtherPeerSocket( void )
1521+
{
1522+
FreeRTOS_Socket_t xSocketToDelete, xParentSocket, xPendingChild;
1523+
ListItem_t xIterator;
1524+
uint16_t usLocalPort = 0x1234;
1525+
1526+
memset( &xSocketToDelete, 0, sizeof( xSocketToDelete ) );
1527+
memset( &xParentSocket, 0, sizeof( xParentSocket ) );
1528+
memset( &xPendingChild, 0, sizeof( xPendingChild ) );
1529+
1530+
xSocketToDelete.ucProtocol = ( uint8_t ) FREERTOS_IPPROTO_TCP;
1531+
xSocketToDelete.u.xTCP.eTCPState = eCLOSED;
1532+
xSocketToDelete.usLocalPort = usLocalPort;
1533+
1534+
xParentSocket.u.xTCP.eTCPState = eTCP_LISTEN;
1535+
xParentSocket.usLocalPort = usLocalPort;
1536+
xParentSocket.u.xTCP.usChildCount = 2;
1537+
xParentSocket.u.xTCP.pxPeerSocket = &xPendingChild;
1538+
1539+
listGET_HEAD_ENTRY_ExpectAndReturn( ( List_t * ) &( xBoundTCPSocketsList ), ( ListItem_t * ) &( xIterator ) );
1540+
1541+
listGET_LIST_ITEM_OWNER_ExpectAndReturn( &xIterator, &xParentSocket );
1542+
1543+
prvTCPSetSocketCount( &xSocketToDelete );
1544+
1545+
TEST_ASSERT_EQUAL( &xPendingChild, xParentSocket.u.xTCP.pxPeerSocket );
1546+
TEST_ASSERT_EQUAL( 1, xParentSocket.u.xTCP.usChildCount );
1547+
}
1548+
1549+
/**
1550+
* @brief The reference must be dropped even when the parent's child count has
1551+
* already reached zero, so the clean-up does not depend on the counter
1552+
* and the parent's own book-keeping staying in step.
1553+
*/
1554+
void test_prvTCPSetSocketCount_NotListeningSock_ClearsPeerSocketZeroChildCount( void )
1555+
{
1556+
FreeRTOS_Socket_t xSocketToDelete, xParentSocket;
1557+
ListItem_t xIterator;
1558+
uint16_t usLocalPort = 0x1234;
1559+
1560+
memset( &xSocketToDelete, 0, sizeof( xSocketToDelete ) );
1561+
memset( &xParentSocket, 0, sizeof( xParentSocket ) );
1562+
1563+
xSocketToDelete.ucProtocol = ( uint8_t ) FREERTOS_IPPROTO_TCP;
1564+
xSocketToDelete.u.xTCP.eTCPState = eCLOSED;
1565+
xSocketToDelete.usLocalPort = usLocalPort;
1566+
1567+
xParentSocket.u.xTCP.eTCPState = eTCP_LISTEN;
1568+
xParentSocket.usLocalPort = usLocalPort;
1569+
xParentSocket.u.xTCP.usChildCount = 0;
1570+
xParentSocket.u.xTCP.pxPeerSocket = &xSocketToDelete;
1571+
1572+
listGET_HEAD_ENTRY_ExpectAndReturn( ( List_t * ) &( xBoundTCPSocketsList ), ( ListItem_t * ) &( xIterator ) );
1573+
1574+
listGET_LIST_ITEM_OWNER_ExpectAndReturn( &xIterator, &xParentSocket );
1575+
1576+
listGET_NEXT_ExpectAndReturn( &( xIterator ), ( ListItem_t * ) &( xBoundTCPSocketsList.xListEnd ) );
1577+
1578+
prvTCPSetSocketCount( &xSocketToDelete );
1579+
1580+
TEST_ASSERT_EQUAL( NULL, xParentSocket.u.xTCP.pxPeerSocket );
1581+
TEST_ASSERT_EQUAL( 0, xParentSocket.u.xTCP.usChildCount );
1582+
}
1583+
14821584
/**
14831585
* @brief Invalid protocol.
14841586
*/

‎test/unit-test/FreeRTOS_TCP_State_Handling/FreeRTOS_TCP_State_Handling_utest.c‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,63 @@ void test_prvTCPSocketCopy_BindError( void )
18561856
TEST_ASSERT_EQUAL( pxSocket->u.xTCP.pxPeerSocket, &MockReturnSocket );
18571857
}
18581858

1859+
/**
1860+
* @brief A child which fails to bind is freed again before this function
1861+
* returns. It must not be counted, nor left reachable from the parent:
1862+
* an unbound child has no local port, so 'prvTCPSetSocketCount()' could
1863+
* not find the parent in order to clear the reference again.
1864+
*/
1865+
void test_prvTCPSocketCopy_BindError_DoesNotLinkChild( void )
1866+
{
1867+
BaseType_t Result = pdFALSE;
1868+
1869+
FreeRTOS_Socket_t MockReturnSocket;
1870+
1871+
pxSocket = &xSocket;
1872+
1873+
pxSocket->usLocalPort = 22;
1874+
pxSocket->u.xTCP.pxPeerSocket = NULL;
1875+
pxSocket->u.xTCP.usChildCount = 0;
1876+
1877+
FreeRTOS_GetLocalAddress_ExpectAndReturn( pxSocket, NULL, pdTRUE );
1878+
FreeRTOS_GetLocalAddress_IgnoreArg_pxAddress();
1879+
vSocketBind_ExpectAnyArgsAndReturn( 1 );
1880+
vSocketClose_ExpectAnyArgsAndReturn( NULL );
1881+
1882+
Result = prvTCPSocketCopy( &MockReturnSocket, pxSocket );
1883+
1884+
TEST_ASSERT_EQUAL( pdFALSE, Result );
1885+
TEST_ASSERT_EQUAL( NULL, pxSocket->u.xTCP.pxPeerSocket );
1886+
TEST_ASSERT_EQUAL( 0, pxSocket->u.xTCP.usChildCount );
1887+
}
1888+
1889+
/**
1890+
* @brief Once the bind has succeeded the child is counted and stored in the
1891+
* listening parent, so that 'FreeRTOS_accept()' can find it.
1892+
*/
1893+
void test_prvTCPSocketCopy_BindSuccess_LinksChild( void )
1894+
{
1895+
BaseType_t Result = pdFALSE;
1896+
1897+
FreeRTOS_Socket_t MockReturnSocket;
1898+
1899+
pxSocket = &xSocket;
1900+
1901+
pxSocket->usLocalPort = 22;
1902+
pxSocket->u.xTCP.pxPeerSocket = NULL;
1903+
pxSocket->u.xTCP.usChildCount = 0;
1904+
1905+
FreeRTOS_GetLocalAddress_ExpectAndReturn( pxSocket, NULL, pdTRUE );
1906+
FreeRTOS_GetLocalAddress_IgnoreArg_pxAddress();
1907+
vSocketBind_ExpectAnyArgsAndReturn( 0 );
1908+
1909+
Result = prvTCPSocketCopy( &MockReturnSocket, pxSocket );
1910+
1911+
TEST_ASSERT_EQUAL( pdTRUE, Result );
1912+
TEST_ASSERT_EQUAL( &MockReturnSocket, pxSocket->u.xTCP.pxPeerSocket );
1913+
TEST_ASSERT_EQUAL( 1, pxSocket->u.xTCP.usChildCount );
1914+
}
1915+
18591916
/**
18601917
* @brief Test for FreeRTOS_GetTCPStateName function.
18611918
*/

0 commit comments

Comments
 (0)