Skip to content

Commit 0a8ad98

Browse files
tony-josi-awstony-josi-aws
authored
Handle buffer allocation failure in prvCreateSectors (#1152)
* Catch allocation failure in prvCreateSectors * Fix unit tests * Fix UTs 2 * Fix formatting
1 parent f5cbeb5 commit 0a8ad98

16 files changed

+447
-121
lines changed

‎source/FreeRTOS_TCP_State_Handling_IPv4.c

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ FreeRTOS_Socket_t * prvHandleListen_IPV4( FreeRTOS_Socket_t * pxSocket,
8181
FreeRTOS_Socket_t * pxReturn = NULL;
8282
uint32_t ulInitialSequenceNumber = 0U;
8383
const NetworkEndPoint_t * pxEndpoint = NULL;
84+
BaseType_t xIsNewSocket = pdFALSE;
8485

8586
if( ( pxSocket != NULL ) && ( pxNetworkBuffer != NULL ) )
8687
{
@@ -155,6 +156,7 @@ FreeRTOS_Socket_t * prvHandleListen_IPV4( FreeRTOS_Socket_t * pxSocket,
155156
* socket to the new socket. Only the binding might fail (due to
156157
* lack of resources). */
157158
pxReturn = pxNewSocket;
159+
xIsNewSocket = pdTRUE;
158160
}
159161
else
160162
{
@@ -166,45 +168,62 @@ FreeRTOS_Socket_t * prvHandleListen_IPV4( FreeRTOS_Socket_t * pxSocket,
166168

167169
if( ( ulInitialSequenceNumber != 0U ) && ( pxReturn != NULL ) )
168170
{
169-
size_t xCopyLength;
171+
do
172+
{
173+
size_t xCopyLength;
174+
BaseType_t xReturnCreateWindow;
170175

171-
/* Map the byte stream onto the ProtocolHeaders_t for easy access to the fields. */
176+
/* Map the byte stream onto the ProtocolHeaders_t for easy access to the fields. */
172177

173-
/* MISRA Ref 11.3.1 [Misaligned access] */
174-
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
175-
/* coverity[misra_c_2012_rule_11_3_violation] */
176-
const ProtocolHeaders_t * pxProtocolHeaders = ( ( const ProtocolHeaders_t * )
177-
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) );
178+
/* MISRA Ref 11.3.1 [Misaligned access] */
179+
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
180+
/* coverity[misra_c_2012_rule_11_3_violation] */
181+
const ProtocolHeaders_t * pxProtocolHeaders = ( ( const ProtocolHeaders_t * )
182+
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) );
178183

179-
/* The endpoint in network buffer must be valid in this condition. */
180-
pxReturn->pxEndPoint = pxNetworkBuffer->pxEndPoint;
181-
pxReturn->bits.bIsIPv6 = pdFALSE_UNSIGNED;
182-
pxReturn->u.xTCP.usRemotePort = FreeRTOS_htons( pxTCPPacket->xTCPHeader.usSourcePort );
183-
pxReturn->u.xTCP.xRemoteIP.ulIP_IPv4 = FreeRTOS_htonl( pxTCPPacket->xIPHeader.ulSourceIPAddress );
184-
pxReturn->u.xTCP.xTCPWindow.ulOurSequenceNumber = ulInitialSequenceNumber;
184+
/* The endpoint in network buffer must be valid in this condition. */
185+
pxReturn->pxEndPoint = pxNetworkBuffer->pxEndPoint;
186+
pxReturn->bits.bIsIPv6 = pdFALSE_UNSIGNED;
187+
pxReturn->u.xTCP.usRemotePort = FreeRTOS_htons( pxTCPPacket->xTCPHeader.usSourcePort );
188+
pxReturn->u.xTCP.xRemoteIP.ulIP_IPv4 = FreeRTOS_htonl( pxTCPPacket->xIPHeader.ulSourceIPAddress );
189+
pxReturn->u.xTCP.xTCPWindow.ulOurSequenceNumber = ulInitialSequenceNumber;
185190

186-
/* Here is the SYN action. */
187-
pxReturn->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
188-
prvSocketSetMSS( pxReturn );
191+
/* Here is the SYN action. */
192+
pxReturn->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
193+
prvSocketSetMSS( pxReturn );
189194

190-
prvTCPCreateWindow( pxReturn );
195+
xReturnCreateWindow = prvTCPCreateWindow( pxReturn );
191196

192-
vTCPStateChange( pxReturn, eSYN_FIRST );
197+
/* Did allocating TCP sectors fail? */
198+
if( xReturnCreateWindow != pdPASS )
199+
{
200+
/* Close the socket if it was newly created. */
201+
if( xIsNewSocket == pdTRUE )
202+
{
203+
vSocketClose( pxReturn );
204+
}
193205

194-
/* Make a copy of the header up to the TCP header. It is needed later
195-
* on, whenever data must be sent to the peer. */
196-
if( pxNetworkBuffer->xDataLength > sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket ) )
197-
{
198-
xCopyLength = sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket );
199-
}
200-
else
201-
{
202-
xCopyLength = pxNetworkBuffer->xDataLength;
203-
}
206+
pxReturn = NULL;
207+
break;
208+
}
209+
210+
vTCPStateChange( pxReturn, eSYN_FIRST );
211+
212+
/* Make a copy of the header up to the TCP header. It is needed later
213+
* on, whenever data must be sent to the peer. */
214+
if( pxNetworkBuffer->xDataLength > sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket ) )
215+
{
216+
xCopyLength = sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket );
217+
}
218+
else
219+
{
220+
xCopyLength = pxNetworkBuffer->xDataLength;
221+
}
204222

205-
( void ) memcpy( ( void * ) pxReturn->u.xTCP.xPacket.u.ucLastPacket,
206-
( const void * ) pxNetworkBuffer->pucEthernetBuffer,
207-
xCopyLength );
223+
( void ) memcpy( ( void * ) pxReturn->u.xTCP.xPacket.u.ucLastPacket,
224+
( const void * ) pxNetworkBuffer->pucEthernetBuffer,
225+
xCopyLength );
226+
} while( ipFALSE_BOOL );
208227
}
209228

210229
return pxReturn;

‎source/FreeRTOS_TCP_State_Handling_IPv6.c

Lines changed: 54 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ FreeRTOS_Socket_t * prvHandleListen_IPV6( FreeRTOS_Socket_t * pxSocket,
7979
FreeRTOS_Socket_t * pxReturn = NULL;
8080
uint32_t ulInitialSequenceNumber = 0;
8181
BaseType_t xHasSequence = pdFALSE;
82+
BaseType_t xIsNewSocket = pdFALSE;
8283

8384
if( ( pxSocket != NULL ) && ( pxNetworkBuffer != NULL ) )
8485
{
@@ -150,6 +151,7 @@ FreeRTOS_Socket_t * prvHandleListen_IPV6( FreeRTOS_Socket_t * pxSocket,
150151
* socket to the new socket. Only the binding might fail (due to
151152
* lack of resources). */
152153
pxReturn = pxNewSocket;
154+
xIsNewSocket = pdTRUE;
153155
}
154156
else
155157
{
@@ -161,50 +163,67 @@ FreeRTOS_Socket_t * prvHandleListen_IPV6( FreeRTOS_Socket_t * pxSocket,
161163

162164
if( ( xHasSequence != pdFALSE ) && ( pxReturn != NULL ) )
163165
{
164-
size_t xCopyLength;
165-
const IPHeader_IPv6_t * pxIPHeader_IPv6;
166+
do
167+
{
168+
size_t xCopyLength;
169+
const IPHeader_IPv6_t * pxIPHeader_IPv6;
170+
BaseType_t xReturnCreateWindow;
166171

167-
/* Map the byte stream onto the ProtocolHeaders_t for easy access to the fields. */
172+
/* Map the byte stream onto the ProtocolHeaders_t for easy access to the fields. */
168173

169-
/* MISRA Ref 11.3.1 [Misaligned access] */
170-
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
171-
/* coverity[misra_c_2012_rule_11_3_violation] */
172-
const ProtocolHeaders_t * pxProtocolHeaders = ( ( const ProtocolHeaders_t * )
173-
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) );
174+
/* MISRA Ref 11.3.1 [Misaligned access] */
175+
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
176+
/* coverity[misra_c_2012_rule_11_3_violation] */
177+
const ProtocolHeaders_t * pxProtocolHeaders = ( ( const ProtocolHeaders_t * )
178+
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + uxIPHeaderSizePacket( pxNetworkBuffer ) ] ) );
174179

175-
pxReturn->pxEndPoint = pxNetworkBuffer->pxEndPoint;
176-
pxReturn->bits.bIsIPv6 = pdTRUE_UNSIGNED;
180+
pxReturn->pxEndPoint = pxNetworkBuffer->pxEndPoint;
181+
pxReturn->bits.bIsIPv6 = pdTRUE_UNSIGNED;
177182

178-
/* MISRA Ref 11.3.1 [Misaligned access] */
179-
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
180-
/* coverity[misra_c_2012_rule_11_3_violation] */
181-
pxIPHeader_IPv6 = ( ( const IPHeader_IPv6_t * ) &( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER ] ) );
182-
pxReturn->u.xTCP.usRemotePort = FreeRTOS_ntohs( pxTCPPacket->xTCPHeader.usSourcePort );
183-
( void ) memcpy( pxReturn->u.xTCP.xRemoteIP.xIP_IPv6.ucBytes, pxIPHeader_IPv6->xSourceAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
184-
pxReturn->u.xTCP.xTCPWindow.ulOurSequenceNumber = ulInitialSequenceNumber;
183+
/* MISRA Ref 11.3.1 [Misaligned access] */
184+
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
185+
/* coverity[misra_c_2012_rule_11_3_violation] */
186+
pxIPHeader_IPv6 = ( ( const IPHeader_IPv6_t * ) &( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER ] ) );
187+
pxReturn->u.xTCP.usRemotePort = FreeRTOS_ntohs( pxTCPPacket->xTCPHeader.usSourcePort );
188+
( void ) memcpy( pxReturn->u.xTCP.xRemoteIP.xIP_IPv6.ucBytes, pxIPHeader_IPv6->xSourceAddress.ucBytes, ipSIZE_OF_IPv6_ADDRESS );
189+
pxReturn->u.xTCP.xTCPWindow.ulOurSequenceNumber = ulInitialSequenceNumber;
185190

186-
/* Here is the SYN action. */
187-
pxReturn->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
188-
prvSocketSetMSS( pxReturn );
191+
/* Here is the SYN action. */
192+
pxReturn->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
193+
prvSocketSetMSS( pxReturn );
189194

190-
prvTCPCreateWindow( pxReturn );
195+
xReturnCreateWindow = prvTCPCreateWindow( pxReturn );
191196

192-
vTCPStateChange( pxReturn, eSYN_FIRST );
197+
/* Did allocating TCP sectors fail? */
198+
if( xReturnCreateWindow != pdPASS )
199+
{
200+
/* Close the socket if it was newly created. */
201+
if( xIsNewSocket == pdTRUE )
202+
{
203+
vSocketClose( pxReturn );
204+
}
193205

194-
/* Make a copy of the header up to the TCP header. It is needed later
195-
* on, whenever data must be sent to the peer. */
196-
if( pxNetworkBuffer->xDataLength > sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket ) )
197-
{
198-
xCopyLength = sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket );
199-
}
200-
else
201-
{
202-
xCopyLength = pxNetworkBuffer->xDataLength;
203-
}
206+
pxReturn = NULL;
207+
break;
208+
}
209+
210+
vTCPStateChange( pxReturn, eSYN_FIRST );
211+
212+
/* Make a copy of the header up to the TCP header. It is needed later
213+
* on, whenever data must be sent to the peer. */
214+
if( pxNetworkBuffer->xDataLength > sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket ) )
215+
{
216+
xCopyLength = sizeof( pxReturn->u.xTCP.xPacket.u.ucLastPacket );
217+
}
218+
else
219+
{
220+
xCopyLength = pxNetworkBuffer->xDataLength;
221+
}
204222

205-
( void ) memcpy( ( void * ) pxReturn->u.xTCP.xPacket.u.ucLastPacket,
206-
( const void * ) pxNetworkBuffer->pucEthernetBuffer,
207-
xCopyLength );
223+
( void ) memcpy( ( void * ) pxReturn->u.xTCP.xPacket.u.ucLastPacket,
224+
( const void * ) pxNetworkBuffer->pucEthernetBuffer,
225+
xCopyLength );
226+
} while( ipFALSE_BOOL );
208227
}
209228

210229
return pxReturn;

‎source/FreeRTOS_TCP_Transmission.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -452,8 +452,9 @@
452452
* (in FreeRTOS_TCP_WIN.c) needs to know them, along with the Maximum Segment
453453
* Size (MSS).
454454
*/
455-
void prvTCPCreateWindow( FreeRTOS_Socket_t * pxSocket )
455+
BaseType_t prvTCPCreateWindow( FreeRTOS_Socket_t * pxSocket )
456456
{
457+
BaseType_t xReturn;
457458
uint32_t ulRxWindowSize = ( uint32_t ) pxSocket->u.xTCP.uxRxWinSize;
458459
uint32_t ulTxWindowSize = ( uint32_t ) pxSocket->u.xTCP.uxTxWinSize;
459460

@@ -466,13 +467,15 @@
466467
( unsigned ) pxSocket->u.xTCP.uxRxStreamSize ) );
467468
}
468469

469-
vTCPWindowCreate(
470+
xReturn = xTCPWindowCreate(
470471
&pxSocket->u.xTCP.xTCPWindow,
471472
ulRxWindowSize * ipconfigTCP_MSS,
472473
ulTxWindowSize * ipconfigTCP_MSS,
473474
pxSocket->u.xTCP.xTCPWindow.rx.ulCurrentSequenceNumber,
474475
pxSocket->u.xTCP.xTCPWindow.ulOurSequenceNumber,
475476
( uint32_t ) pxSocket->u.xTCP.usMSS );
477+
478+
return xReturn;
476479
}
477480
/*-----------------------------------------------------------*/
478481

‎source/FreeRTOS_TCP_Transmission_IPv4.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,10 @@ BaseType_t prvTCPPrepareConnect_IPV4( FreeRTOS_Socket_t * pxSocket )
457457
/* The initial sequence numbers at our side are known. Later
458458
* vTCPWindowInit() will be called to fill in the peer's sequence numbers, but
459459
* first wait for a SYN+ACK reply. */
460-
prvTCPCreateWindow( pxSocket );
460+
if( prvTCPCreateWindow( pxSocket ) != pdTRUE )
461+
{
462+
xReturn = pdFALSE;
463+
}
461464
}
462465

463466
return xReturn;

‎source/FreeRTOS_TCP_Transmission_IPv6.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,10 @@ BaseType_t prvTCPPrepareConnect_IPV6( FreeRTOS_Socket_t * pxSocket )
468468
/* The initial sequence numbers at our side are known. Later
469469
* vTCPWindowInit() will be called to fill in the peer's sequence numbers, but
470470
* first wait for a SYN+ACK reply. */
471-
prvTCPCreateWindow( pxSocket );
471+
if( prvTCPCreateWindow( pxSocket ) != pdTRUE )
472+
{
473+
xReturn = pdFAIL;
474+
}
472475
}
473476
else
474477
{

‎source/FreeRTOS_TCP_WIN.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -777,20 +777,22 @@
777777
* @param[in] ulSequenceNumber The first sequence number.
778778
* @param[in] ulMSS The MSS of the connection.
779779
*/
780-
void vTCPWindowCreate( TCPWindow_t * pxWindow,
781-
uint32_t ulRxWindowLength,
782-
uint32_t ulTxWindowLength,
783-
uint32_t ulAckNumber,
784-
uint32_t ulSequenceNumber,
785-
uint32_t ulMSS )
780+
BaseType_t xTCPWindowCreate( TCPWindow_t * pxWindow,
781+
uint32_t ulRxWindowLength,
782+
uint32_t ulTxWindowLength,
783+
uint32_t ulAckNumber,
784+
uint32_t ulSequenceNumber,
785+
uint32_t ulMSS )
786786
{
787+
BaseType_t xReturn = pdPASS;
788+
787789
/* Create and initialize a window. */
788790

789791
#if ( ipconfigUSE_TCP_WIN == 1 )
790792
{
791793
if( xTCPSegments == NULL )
792794
{
793-
( void ) prvCreateSectors();
795+
xReturn = prvCreateSectors();
794796
}
795797

796798
vListInitialise( &( pxWindow->xTxSegments ) );
@@ -804,14 +806,16 @@
804806

805807
if( xTCPWindowLoggingLevel != 0 )
806808
{
807-
FreeRTOS_debug_printf( ( "vTCPWindowCreate: for WinLen = Rx/Tx: %u/%u\n",
809+
FreeRTOS_debug_printf( ( "xTCPWindowCreate: for WinLen = Rx/Tx: %u/%u\n",
808810
( unsigned ) ulRxWindowLength, ( unsigned ) ulTxWindowLength ) );
809811
}
810812

811813
pxWindow->xSize.ulRxWindowLength = ulRxWindowLength;
812814
pxWindow->xSize.ulTxWindowLength = ulTxWindowLength;
813815

814816
vTCPWindowInit( pxWindow, ulAckNumber, ulSequenceNumber, ulMSS );
817+
818+
return xReturn;
815819
}
816820
/*-----------------------------------------------------------*/
817821

‎source/include/FreeRTOS_TCP_Transmission.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ BaseType_t prvTCPPrepareConnect_IPV6( FreeRTOS_Socket_t * pxSocket );
105105
/*
106106
* Initialise the data structures which keep track of the TCP windowing system.
107107
*/
108-
void prvTCPCreateWindow( FreeRTOS_Socket_t * pxSocket );
108+
BaseType_t prvTCPCreateWindow( FreeRTOS_Socket_t * pxSocket );
109109

110110
/*
111111
* Set the initial properties in the options fields, like the preferred

‎source/include/FreeRTOS_TCP_WIN.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,12 @@ typedef struct xTCP_WINDOW
148148
*=============================================================================*/
149149

150150
/* Create and initialize a window */
151-
void vTCPWindowCreate( TCPWindow_t * pxWindow,
152-
uint32_t ulRxWindowLength,
153-
uint32_t ulTxWindowLength,
154-
uint32_t ulAckNumber,
155-
uint32_t ulSequenceNumber,
156-
uint32_t ulMSS );
151+
BaseType_t xTCPWindowCreate( TCPWindow_t * pxWindow,
152+
uint32_t ulRxWindowLength,
153+
uint32_t ulTxWindowLength,
154+
uint32_t ulAckNumber,
155+
uint32_t ulSequenceNumber,
156+
uint32_t ulMSS );
157157

158158
/* Destroy a window (always returns NULL)
159159
* It will free some resources: a collection of segments */

0 commit comments

Comments
 (0)