Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[IPv6] Fix coverity #933

Merged
merged 6 commits into from
Jun 26, 2023
Merged

[IPv6] Fix coverity #933

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d56a46b
Fix coverity
ActoryOu Jun 20, 2023
4045036
Fix UT
ActoryOu Jun 20, 2023
84bf4ab
Fix build combination
ActoryOu Jun 20, 2023
62f3e18
Suppress rule 17.2
ActoryOu Jun 21, 2023
e872281
Merge branch 'dev/IPv6_integration' into MISRA_0620
ActoryOu Jun 26, 2023
5c71030
Fix 11.3 and 15.7 violations
ActoryOu Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions source/FreeRTOS_ARP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,11 +476,11 @@ BaseType_t xIsIPInARPCache( uint32_t ulAddressToLookup )
*
* @return pdTRUE if the packet needs ARP resolution, pdFALSE otherwise.
*/
BaseType_t xCheckRequiresARPResolution( NetworkBufferDescriptor_t * pxNetworkBuffer )
BaseType_t xCheckRequiresARPResolution( const NetworkBufferDescriptor_t * pxNetworkBuffer )
{
BaseType_t xNeedsARPResolution = pdFALSE;

switch( uxIPHeaderSizePacket( ( const NetworkBufferDescriptor_t * ) pxNetworkBuffer ) )
switch( uxIPHeaderSizePacket( pxNetworkBuffer ) )
{
#if ( ipconfigUSE_IPv4 != 0 )
case ipSIZE_OF_IPv4_HEADER:
Expand Down
219 changes: 105 additions & 114 deletions source/FreeRTOS_DNS_Cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
/*!
* @brief DNS cache structure instantiation
*/
static DNSCacheRow_t xDNSCache[ ipconfigDNS_CACHE_ENTRIES ] = { 0x0 };
static DNSCacheRow_t xDNSCache[ ipconfigDNS_CACHE_ENTRIES ];

/*!
* @brief indicates the index of a free entry in the cache structure
Expand Down Expand Up @@ -82,11 +82,9 @@
const IPv46_Address_t * pxIP,
uint32_t ulCurrentTimeSeconds );

#if ( ipconfigUSE_DNS_CACHE == 1 )
/** Copy DNS cache entries at xIndex to a linked struct addrinfo. */
static void prvReadDNSCache( BaseType_t uxIndex,
struct freertos_addrinfo ** ppxAddressInfo );
#endif
/** Copy DNS cache entries at xIndex to a linked struct addrinfo. */
static void prvReadDNSCache( BaseType_t uxIndex,
struct freertos_addrinfo ** ppxAddressInfo );

/*-----------------------------------------------------------*/

Expand Down Expand Up @@ -476,76 +474,71 @@
}
/*-----------------------------------------------------------*/

#if ( ipconfigUSE_DNS_CACHE == 1 )

/**
* @brief Copy DNS cache entries at uxIndex to a linked struct addrinfo.
* @param[in] uxIndex The index from where entries must be copied.
* @param[out] ppxAddressInfo Target to store the DNS entries.
*/
static void prvReadDNSCache( BaseType_t uxIndex,
struct freertos_addrinfo ** ppxAddressInfo )
{
size_t uxIPAddressIndex;
size_t uxNumIPAddresses = 1U;
const IPv46_Address_t * pxAddresses;
struct freertos_addrinfo * pxNewAddress = NULL;
struct freertos_addrinfo ** ppxLastAddress = ppxAddressInfo;

#if ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 )
uxNumIPAddresses = ( size_t ) xDNSCache[ uxIndex ].ucNumIPAddresses;
static void prvReadDNSCache( BaseType_t uxIndex,
struct freertos_addrinfo ** ppxAddressInfo )
{
size_t uxIPAddressIndex;
size_t uxNumIPAddresses = 1U;
const IPv46_Address_t * pxAddresses;
struct freertos_addrinfo * pxNewAddress = NULL;
struct freertos_addrinfo ** ppxLastAddress = ppxAddressInfo;

if( uxNumIPAddresses > ( size_t ) ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY )
{
/* Make this a configASSERT()? */
uxNumIPAddresses = ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY;
}
#endif /* ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 ) */
#if ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 )
uxNumIPAddresses = ( size_t ) xDNSCache[ uxIndex ].ucNumIPAddresses;

for( uxIPAddressIndex = 0; uxIPAddressIndex < uxNumIPAddresses; uxIPAddressIndex++ )
if( uxNumIPAddresses > ( size_t ) ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY )
{
pxAddresses = &( xDNSCache[ uxIndex ].xAddresses[ uxIPAddressIndex ] );

switch( pxAddresses->xIs_IPv6 ) /* LCOV_EXCL_BR_LINE - xIs_IPv6 is always either pdFALSE or pdTRUE. */
{
#if ( ipconfigUSE_IPv4 != 0 )
case pdFALSE:
{
const uint8_t * ucBytes = ( const uint8_t * ) &( pxAddresses->xIPAddress.ulIP_IPv4 );
pxNewAddress = pxNew_AddrInfo( xDNSCache[ uxIndex ].pcName, FREERTOS_AF_INET4, ucBytes );
}
break;
#endif /* ( ipconfigUSE_IPv4 != 0 ) */
/* Make this a configASSERT()? */
uxNumIPAddresses = ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY;
}
#endif /* ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 ) */

#if ( ipconfigUSE_IPv6 != 0 )
case pdTRUE:
pxNewAddress = pxNew_AddrInfo( xDNSCache[ uxIndex ].pcName, FREERTOS_AF_INET6, pxAddresses->xIPAddress.xIP_IPv6.ucBytes );
break;
#endif /* ( ipconfigUSE_IPv6 != 0 ) */
for( uxIPAddressIndex = 0; uxIPAddressIndex < uxNumIPAddresses; uxIPAddressIndex++ )
{
pxAddresses = &( xDNSCache[ uxIndex ].xAddresses[ uxIPAddressIndex ] );

default: /* LCOV_EXCL_LINE - xIs_IPv6 is always either pdFALSE or FREERTOS_AF_INET6. */
/* MISRA 16.4 Compliance */
FreeRTOS_debug_printf( ( "prvReadDNSCache: Undefined IP Type \n" ) );
break; /* LCOV_EXCL_LINE - xIs_IPv6 is always either pdFALSE or FREERTOS_AF_INET6. */
}
switch( pxAddresses->xIs_IPv6 ) /* LCOV_EXCL_BR_LINE - xIs_IPv6 is always either pdFALSE or pdTRUE. */
{
#if ( ipconfigUSE_IPv4 != 0 )
case pdFALSE:
{
const uint8_t * ucBytes = ( const uint8_t * ) &( pxAddresses->xIPAddress.ulIP_IPv4 );
pxNewAddress = pxNew_AddrInfo( xDNSCache[ uxIndex ].pcName, FREERTOS_AF_INET4, ucBytes );
}
break;
#endif /* ( ipconfigUSE_IPv4 != 0 ) */

if( pxNewAddress == NULL )
{
/* Malloc must has failed. */
break;
}
#if ( ipconfigUSE_IPv6 != 0 )
case pdTRUE:
pxNewAddress = pxNew_AddrInfo( xDNSCache[ uxIndex ].pcName, FREERTOS_AF_INET6, pxAddresses->xIPAddress.xIP_IPv6.ucBytes );
break;
#endif /* ( ipconfigUSE_IPv6 != 0 ) */

/* Set either 'ppxAddressInfo' or 'pxNewAddress->ai_next'. */
*( ppxLastAddress ) = pxNewAddress;
default: /* LCOV_EXCL_LINE - xIs_IPv6 is always either pdFALSE or FREERTOS_AF_INET6. */
/* MISRA 16.4 Compliance */
FreeRTOS_debug_printf( ( "prvReadDNSCache: Undefined IP Type \n" ) );
break; /* LCOV_EXCL_LINE - xIs_IPv6 is always either pdFALSE or FREERTOS_AF_INET6. */
}

ppxLastAddress = &( pxNewAddress->ai_next );
if( pxNewAddress == NULL )
{
/* Malloc must has failed. */
break;
}

/* Set either 'ppxAddressInfo' or 'pxNewAddress->ai_next'. */
*( ppxLastAddress ) = pxNewAddress;

ppxLastAddress = &( pxNewAddress->ai_next );
}
#endif /* #if( ipconfigUSE_DNS_CACHE == 1 ) */
}
/*-----------------------------------------------------------*/

#if ( ipconfigUSE_DNS_CACHE == 1 )

/**
* @brief Lookup the given hostname in the DNS cache
* @param[in] pcHostName THe host name to lookup
Expand All @@ -554,74 +547,73 @@
* @returns This function returns either a valid IPv4 address, or
* in case of an IPv6 lookup, it will return a non-zero.
*/
uint32_t Prepare_CacheLookup( const char * pcHostName,
BaseType_t xFamily,
struct freertos_addrinfo ** ppxAddressInfo )
{
uint32_t ulIPAddress = 0U;
IPv46_Address_t xIPv46_Address;
uint32_t Prepare_CacheLookup( const char * pcHostName,
BaseType_t xFamily,
struct freertos_addrinfo ** ppxAddressInfo )
{
uint32_t ulIPAddress = 0U;
IPv46_Address_t xIPv46_Address;

switch( xFamily )
{
#if ( ipconfigUSE_IPv4 != 0 )
case FREERTOS_AF_INET:
{
BaseType_t xFound;
switch( xFamily )
{
#if ( ipconfigUSE_IPv4 != 0 )
case FREERTOS_AF_INET:
{
BaseType_t xFound;

xIPv46_Address.xIs_IPv6 = pdFALSE;
xFound = FreeRTOS_ProcessDNSCache( pcHostName, &( xIPv46_Address ), 0, pdTRUE, ppxAddressInfo );
xIPv46_Address.xIs_IPv6 = pdFALSE;
xFound = FreeRTOS_ProcessDNSCache( pcHostName, &( xIPv46_Address ), 0, pdTRUE, ppxAddressInfo );

if( xFound != 0 )
if( xFound != 0 )
{
if( ( ppxAddressInfo != NULL ) && ( *( ppxAddressInfo ) != NULL ) )
{
if( ( ppxAddressInfo != NULL ) && ( *( ppxAddressInfo ) != NULL ) )
{
const struct freertos_sockaddr * sockaddr = ( *( ppxAddressInfo ) )->ai_addr;
const struct freertos_sockaddr * sockaddr = ( *( ppxAddressInfo ) )->ai_addr;

ulIPAddress = sockaddr->sin_address.ulIP_IPv4;
}
}
else
{
/* prvGetHostByName will be called to start a DNS lookup. */
ulIPAddress = sockaddr->sin_address.ulIP_IPv4;
}
}
break;
#endif /* ( ipconfigUSE_IPv4 != 0 ) */

#if ( ipconfigUSE_IPv6 != 0 )
case FREERTOS_AF_INET6:
else
{
BaseType_t xFound;
/* prvGetHostByName will be called to start a DNS lookup. */
}
}
break;
#endif /* ( ipconfigUSE_IPv4 != 0 ) */

xIPv46_Address.xIs_IPv6 = pdTRUE;
xFound = FreeRTOS_ProcessDNSCache( pcHostName, &( xIPv46_Address ), 0, pdTRUE, ppxAddressInfo );
#if ( ipconfigUSE_IPv6 != 0 )
case FREERTOS_AF_INET6:
{
BaseType_t xFound;

if( xFound != 0 )
{
if( ( ppxAddressInfo != NULL ) && ( *( ppxAddressInfo ) != NULL ) )
{
/* This function returns either a valid IPv4 address, or
* in case of an IPv6 lookup, it will return a non-zero */
ulIPAddress = 1U;
}
}
else
xIPv46_Address.xIs_IPv6 = pdTRUE;
xFound = FreeRTOS_ProcessDNSCache( pcHostName, &( xIPv46_Address ), 0, pdTRUE, ppxAddressInfo );

if( xFound != 0 )
{
if( ( ppxAddressInfo != NULL ) && ( *( ppxAddressInfo ) != NULL ) )
{
/* prvGetHostByName will be called to start a DNS lookup. */
/* This function returns either a valid IPv4 address, or
* in case of an IPv6 lookup, it will return a non-zero */
ulIPAddress = 1U;
}
}
break;
#endif /* ( ipconfigUSE_IPv6 != 0 ) */

default:
/* MISRA 16.4 Compliance */
FreeRTOS_debug_printf( ( "Prepare_CacheLookup: Undefined xFamily \n" ) );
break;
}

return ulIPAddress;
else
{
/* prvGetHostByName will be called to start a DNS lookup. */
}
}
break;
#endif /* ( ipconfigUSE_IPv6 != 0 ) */

default:
/* MISRA 16.4 Compliance */
FreeRTOS_debug_printf( ( "Prepare_CacheLookup: Undefined xFamily \n" ) );
break;
}
#endif /* ( ipconfigUSE_DNS_CACHE == 1 ) */

return ulIPAddress;
}
/*-----------------------------------------------------------*/

#if ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 )
Expand Down Expand Up @@ -688,5 +680,4 @@
#endif /* if ( ipconfigDNS_CACHE_ADDRESSES_PER_ENTRY > 1 ) */
/*-----------------------------------------------------------*/


#endif /* if ( ( ipconfigUSE_DNS != 0 ) && ( ipconfigUSE_DNS_CACHE == 1 ) ) */
13 changes: 10 additions & 3 deletions source/FreeRTOS_IP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,13 +519,20 @@ static void prvIPTask_Initialise( void )
/* Mark the timer as inactive since we are not waiting on any ARP resolution as of now. */
vIPSetARPResolutionTimerEnableState( pdFALSE );

#if ( ipconfigDNS_USE_CALLBACKS != 0 )
#if ( ( ipconfigDNS_USE_CALLBACKS != 0 ) && ( ipconfigUSE_DNS != 0 ) )
{
/* The following function is declared in FreeRTOS_DNS.c and 'private' to
/* The following function is declared in FreeRTOS_DNS.c and 'private' to
* this library */
vDNSInitialise();
}
#endif /* ipconfigDNS_USE_CALLBACKS != 0 */
#endif /* ( ipconfigDNS_USE_CALLBACKS != 0 ) && ( ipconfigUSE_DNS != 0 ) */

#if ( ( ipconfigUSE_DNS_CACHE != 0 ) && ( ipconfigUSE_DNS != 0 ) )
{
/* Clear the DNS cache once only. */
FreeRTOS_dnsclear();
}
#endif /* ( ( ipconfigUSE_DNS_CACHE != 0 ) && ( ipconfigUSE_DNS != 0 ) ) */

/* Initialisation is complete and events can now be processed. */
xIPTaskInitialised = pdTRUE;
Expand Down
2 changes: 1 addition & 1 deletion source/FreeRTOS_IP_Utils.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ static BaseType_t prvChecksumProtocolChecks( size_t uxBufferLength,
}
else
{
uxOptionsLength = ( size_t ) ( ( ucLength - 5U ) << 2U );
uxOptionsLength = ( ( ( size_t ) ucLength - 5U ) << 2U );

pxSet->uxProtocolHeaderLength = ipSIZE_OF_TCP_HEADER + uxOptionsLength;
#if ( ipconfigHAS_DEBUG_PRINTF != 0 )
Expand Down
2 changes: 1 addition & 1 deletion source/FreeRTOS_ND.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@

if( eMyType == eIPType )
{
eReturn = prvNDCacheLookup( pxIPAddress, pxMACAddress, &pxEndPoint );
eReturn = prvNDCacheLookup( pxIPAddress, pxMACAddress, ppxEndPoint );
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion source/FreeRTOS_RA.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@
size_t uxPrefixLength = ( size_t ) pucBytes[ uxIndex + 1U ];
size_t uxLength = uxPrefixLength * 8U;

if( uxPrefixLength == 0 )
if( uxPrefixLength == 0U )
{
/* According to RFC 4861, length of the option value 0 is invalid. Hence returning from here */
FreeRTOS_printf( ( "RA: Invalid length of the option value as zero. " ) );
Expand Down
20 changes: 10 additions & 10 deletions source/FreeRTOS_Routing.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,8 +740,8 @@ struct xIPv6_Couple
NetworkEndPoint_t * pxEndPoint;
NetworkEndPoint_t * pxReturn = NULL;
/* endpoints found for IP-type, IP-address, and MAC-address. */
NetworkEndPoint_t * pxFound[ rMATCH_COUNT ] = { NULL, NULL, NULL };
BaseType_t xCount[ rMATCH_COUNT ] = { 0, 0, 0 };
NetworkEndPoint_t * pxFound[ rMATCH_COUNT ] = { NULL, NULL, NULL, NULL };
BaseType_t xCount[ rMATCH_COUNT ] = { 0, 0, 0, 0 };
BaseType_t xIndex;
BaseType_t xIsIPv6 = ( usFrameType == ipIPv6_FRAME_TYPE ) ? pdTRUE : pdFALSE;
BaseType_t xGatewayTarget = pdFALSE;
Expand Down Expand Up @@ -821,9 +821,9 @@ struct xIPv6_Couple
break;
#endif /* ( ipconfigUSE_IPv6 != 0 ) */

#if ( ipconfigUSE_IPv4 != 0 )
case ( BaseType_t ) pdFALSE:

case ( BaseType_t ) pdFALSE:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we decided not to do these kind of declaration. @tony-josi-aws to take a look.

Copy link
Member Author

@ActoryOu ActoryOu Jun 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is detected as DEADCODE by Coverity. xIsIPv6 is set to either pdTRUE or pdFALSE in the top of this function. It's impossible to enter default case when IPv4/IPv6 are both enabled (which we use in coverity scanning). But it's still possible in differnt configuration (for example, IPv4 only). So we need to keep it in the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only alternative is to take a MISRA exception here, but I believe this looks better than taking exception.

default:
#if ( ipconfigUSE_IPv4 != 0 )
if( pxEndPoint->ipv4_settings.ulIPAddress == pxIPAddressTo->ulIP_IPv4 )
{
pxFound[ rMATCH_IP_ADDR ] = pxEndPoint;
Expand All @@ -833,12 +833,9 @@ struct xIPv6_Couple
{
/* do nothing, coverity happy */
}
break;
#endif /* ( ipconfigUSE_IPv4 != 0 ) */
#endif /* ( ipconfigUSE_IPv4 != 0 ) */

default: /* LCOV_EXCL_LINE */
/* MISRA 16.4 Compliance */
break; /* LCOV_EXCL_LINE */
break;
}

if( xSameMACAddress == pdTRUE )
Expand Down Expand Up @@ -1522,6 +1519,9 @@ const char * pcEndpointName( const NetworkEndPoint_t * pxEndPoint,

default:
/* MISRA 16.4 Compliance */
/* MISRA Ref 21.6.1 [snprintf and logging] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-216 */
/* coverity[misra_c_2012_rule_21_6_violation] */
( void ) snprintf( pcBuffer, uxSize, "NULL" );
break;
}
Expand Down
Loading