From 0e39b8b20121650d6ef44570b651aeb314295566 Mon Sep 17 00:00:00 2001 From: Holden Ramsey Date: Tue, 8 Sep 2026 19:33:20 -0400 Subject: [PATCH 1/2] feat: add common packet filtering for network drivers Add eConsiderPacketForProcessing and share IPv4/IPv6 admission rules with IP-task processing while preserving existing loopback behavior. Document the driver helper and cover framing, endpoint selection, and policy delegation in unit tests. --- source/FreeRTOS_IP.c | 125 +++++++++ source/FreeRTOS_IPv4.c | 262 ++++++++++-------- source/FreeRTOS_IPv6.c | 151 ++++++---- source/include/FreeRTOSIPConfigDefaults.h | 5 + source/include/FreeRTOS_IP_Private.h | 8 + source/include/FreeRTOS_IPv4_Private.h | 5 + source/include/FreeRTOS_IPv6.h | 11 +- .../unit-test/FreeRTOS_IP/FreeRTOS_IP_utest.c | 30 ++ .../FreeRTOS_IP_DiffConfig3_utest.c | 167 +++++++++++ 9 files changed, 576 insertions(+), 188 deletions(-) diff --git a/source/FreeRTOS_IP.c b/source/FreeRTOS_IP.c index dee5c62ba..0845f7e16 100644 --- a/source/FreeRTOS_IP.c +++ b/source/FreeRTOS_IP.c @@ -1671,6 +1671,131 @@ eFrameProcessingResult_t eConsiderFrameForProcessing( const uint8_t * const pucE } /*-----------------------------------------------------------*/ +/** + * @brief Decide whether a packet should be processed based on its IP header + * and the endpoint selected by the network driver. + * + * @param[in] pxNetworkBuffer The network buffer containing the packet. Its + * pxEndPoint member must already be assigned. + * + * @return eProcessBuffer when the packet passes the checks, or + * eReleaseBuffer when it should be discarded. + */ +eFrameProcessingResult_t eConsiderPacketForProcessing( const NetworkBufferDescriptor_t * const pxNetworkBuffer ) +{ + eFrameProcessingResult_t eReturn = eReleaseBuffer; + + do + { + const EthernetHeader_t * pxEthernetHeader; + const NetworkEndPoint_t * pxEndPoint; + + if( pxNetworkBuffer == NULL ) + { + break; + } + + if( pxNetworkBuffer->pucEthernetBuffer == NULL ) + { + break; + } + + if( pxNetworkBuffer->pxEndPoint == NULL ) + { + break; + } + + if( pxNetworkBuffer->xDataLength < sizeof( EthernetHeader_t ) ) + { + break; + } + + /* 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] */ + pxEthernetHeader = ( const EthernetHeader_t * ) pxNetworkBuffer->pucEthernetBuffer; + pxEndPoint = pxNetworkBuffer->pxEndPoint; + + #if ipconfigIS_ENABLED( ipconfigUSE_IPv4 ) + if( pxEthernetHeader->usFrameType == ipARP_FRAME_TYPE ) + { + eReturn = eProcessBuffer; + break; + } + + if( pxEthernetHeader->usFrameType == ipIPv4_FRAME_TYPE ) + { + const IPPacket_t * pxIPPacket; + const IPHeader_t * pxIPHeader; + size_t uxHeaderLength; + + /* pxEndPoint was validated above, so inspect its address + * family directly rather than repeating the NULL check in + * ENDPOINT_IS_IPv4(). */ + if( pxEndPoint->bits.bIPv6 != pdFALSE_UNSIGNED ) + { + break; + } + + if( pxNetworkBuffer->xDataLength < sizeof( IPPacket_t ) ) + { + break; + } + + /* 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] */ + pxIPPacket = ( const IPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer; + pxIPHeader = &( pxIPPacket->xIPHeader ); + + uxHeaderLength = ( size_t ) ( ( pxIPHeader->ucVersionHeaderLength & 0x0FU ) << 2 ); + + if( uxHeaderLength > ( pxNetworkBuffer->xDataLength - ipSIZE_OF_ETH_HEADER ) ) + { + break; + } + + eReturn = eConsiderIPv4PacketForProcessing( pxIPPacket, pxEndPoint, pdFALSE ); + break; + } + #endif /* ipconfigUSE_IPv4 */ + + #if ipconfigIS_ENABLED( ipconfigUSE_IPv6 ) + if( pxEthernetHeader->usFrameType == ipIPv6_FRAME_TYPE ) + { + const IPPacket_IPv6_t * pxIPv6Packet; + + /* pxEndPoint was validated above, so inspect its address + * family directly rather than repeating the NULL check in + * ENDPOINT_IS_IPv6(). */ + if( pxEndPoint->bits.bIPv6 == pdFALSE_UNSIGNED ) + { + break; + } + + if( pxNetworkBuffer->xDataLength < sizeof( IPPacket_IPv6_t ) ) + { + break; + } + + /* 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] */ + pxIPv6Packet = ( const IPPacket_IPv6_t * ) pxNetworkBuffer->pucEthernetBuffer; + eReturn = eConsiderIPv6PacketForProcessing( &( pxIPv6Packet->xIPHeader ), pxEndPoint, pdFALSE ); + break; + } + #endif /* ipconfigUSE_IPv6 */ + + #if ipconfigIS_ENABLED( ipconfigPROCESS_CUSTOM_ETHERNET_FRAMES ) + eReturn = eProcessBuffer; + #endif + } while( ipFALSE_BOOL ); + + return eReturn; +} +/*-----------------------------------------------------------*/ + /** * @brief Process the Ethernet packet. * diff --git a/source/FreeRTOS_IPv4.c b/source/FreeRTOS_IPv4.c index 80a0dbcf2..a639ccc42 100644 --- a/source/FreeRTOS_IPv4.c +++ b/source/FreeRTOS_IPv4.c @@ -320,110 +320,118 @@ BaseType_t xIsIPv4Loopback( uint32_t ulAddress ) /*-----------------------------------------------------------*/ /** - * @brief Check whether this IPv4 packet is to be allowed or to be dropped. + * @brief Apply IPv4 admission policy shared by the IP task and driver filter. * - * @param[in] pxIPPacket The IP packet under consideration. - * @param[in] pxNetworkBuffer The whole network buffer. - * @param[in] uxHeaderLength The length of the header. + * The caller must provide a complete IPv4 header and an assigned IPv4 endpoint. + * Length, checksum and transport validation remain the caller's responsibility. * + * @param[in] pxIPPacket Packet containing the Ethernet and IPv4 headers. + * @param[in] pxEndPoint Endpoint selected for this packet. + * @param[in] xAllowHostLoopback pdTRUE for the IP task's internal loopback path; + * pdFALSE for network driver filtering. * @return Whether the packet should be processed or dropped. */ -enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * const pxIPPacket, - const struct xNETWORK_BUFFER * const pxNetworkBuffer, - UBaseType_t uxHeaderLength ) +eFrameProcessingResult_t eConsiderIPv4PacketForProcessing( const IPPacket_t * const pxIPPacket, + const NetworkEndPoint_t * const pxEndPoint, + BaseType_t xAllowHostLoopback ) { eFrameProcessingResult_t eReturn = eProcessBuffer; + const IPHeader_t * pxIPHeader = &( pxIPPacket->xIPHeader ); + uint32_t ulDestinationIPAddress = pxIPHeader->ulDestinationIPAddress; + uint32_t ulSourceIPAddress = pxIPHeader->ulSourceIPAddress; + + /* Ensure that the incoming packet is not fragmented because the stack + * doesn't not support IP fragmentation. All but the last fragment coming in will have their + * "more fragments" flag set and the last fragment will have a non-zero offset. + * We need to drop the packet in either of those cases. */ + if( ( ( pxIPHeader->usFragmentOffset & ipFRAGMENT_OFFSET_BIT_MASK ) != 0U ) || ( ( pxIPHeader->usFragmentOffset & ipFRAGMENT_FLAGS_MORE_FRAGMENTS ) != 0U ) ) + { + /* Can not handle, fragmented packet. */ + eReturn = eReleaseBuffer; + } - #if ( ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) || ( ipconfigDRIVER_INCLUDED_RX_IP_CHECKSUM == 0 ) ) - const IPHeader_t * pxIPHeader = &( pxIPPacket->xIPHeader ); - #else - - /* or else, the parameter won't be used and the function will be optimised - * away */ - ( void ) pxIPPacket; - #endif + /* Test if the length of the IP-header is between 20 and 60 bytes, + * and if the IP-version is 4. */ + else if( ( pxIPHeader->ucVersionHeaderLength < ipIPV4_VERSION_HEADER_LENGTH_MIN ) || + ( pxIPHeader->ucVersionHeaderLength > ipIPV4_VERSION_HEADER_LENGTH_MAX ) ) + { + /* Can not handle, unknown or invalid header version. */ + eReturn = eReleaseBuffer; + } + else if( xBadIPv4Loopback( pxIPHeader ) == pdTRUE ) + { + eReturn = eReleaseBuffer; + } + else if( ( xAllowHostLoopback != pdFALSE ) && + ( ( xIsIPv4Loopback( ulDestinationIPAddress ) == pdTRUE ) || + ( xIsIPv4Loopback( ulSourceIPAddress ) == pdTRUE ) ) ) + { + /* Preserve the IP task's internal loopback exception. A driver + * still applies the Ethernet and endpoint checks below. */ + } + else if( memcmp( xBroadcastMACAddress.ucBytes, + pxIPPacket->xEthernetHeader.xSourceAddress.ucBytes, + sizeof( MACAddress_t ) ) == 0 ) + { + /* Ethernet source is a broadcast address. Drop the packet. */ + eReturn = eReleaseBuffer; + } + else if( xIsIPv4Multicast( ulSourceIPAddress ) == pdTRUE ) + { + /* Source is a multicast IP address. Drop the packet in conformity with RFC 1112 section 7.2. */ + eReturn = eReleaseBuffer; + } - #if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) + /* Use ipv4_settings for filtering only after the endpoint is up, + * so that DHCP packets that are exchanged for DHCP (example, DHCP unicast offers) + * are not dropped/filtered. */ + else if( FreeRTOS_IsEndPointUp( pxEndPoint ) != pdFALSE ) { - /* In systems with a very small amount of RAM, it might be advantageous - * to have incoming messages checked earlier, by the network card driver. - * This method may decrease the usage of scarce network buffers. */ - uint32_t ulDestinationIPAddress = pxIPHeader->ulDestinationIPAddress; - uint32_t ulSourceIPAddress = pxIPHeader->ulSourceIPAddress; - /* Get a reference to the endpoint that the packet was assigned to during pxEasyFit() */ - const NetworkEndPoint_t * pxEndPoint = pxNetworkBuffer->pxEndPoint; - - /* Ensure that the incoming packet is not fragmented because the stack - * doesn't not support IP fragmentation. All but the last fragment coming in will have their - * "more fragments" flag set and the last fragment will have a non-zero offset. - * We need to drop the packet in either of those cases. */ - if( ( ( pxIPHeader->usFragmentOffset & ipFRAGMENT_OFFSET_BIT_MASK ) != 0U ) || ( ( pxIPHeader->usFragmentOffset & ipFRAGMENT_FLAGS_MORE_FRAGMENTS ) != 0U ) ) + if( + /* Not destined for the assigned endpoint IPv4 address? */ + ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulIPAddress ) && + /* Also not an IPv4 broadcast address ? */ + ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) && + ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) && + /* And not an IPv4 multicast address ? */ + ( xIsIPv4Multicast( ulDestinationIPAddress ) == pdFALSE ) ) { - /* Can not handle, fragmented packet. */ + /* Packet is not for this node, release it */ eReturn = eReleaseBuffer; } - - /* Test if the length of the IP-header is between 20 and 60 bytes, - * and if the IP-version is 4. */ - else if( ( pxIPHeader->ucVersionHeaderLength < ipIPV4_VERSION_HEADER_LENGTH_MIN ) || - ( pxIPHeader->ucVersionHeaderLength > ipIPV4_VERSION_HEADER_LENGTH_MAX ) ) + /* Is the source address correct? */ + else if( ( ulSourceIPAddress == pxEndPoint->ipv4_settings.ulBroadcastAddress ) || + ( ulSourceIPAddress == FREERTOS_INADDR_BROADCAST ) ) { - /* Can not handle, unknown or invalid header version. */ + /* The source address cannot be broadcast address. Replying to this + * packet may cause network storms. Drop the packet. */ eReturn = eReleaseBuffer; } - else if( ( xIsIPv4Loopback( ulDestinationIPAddress ) == pdTRUE ) || - ( xIsIPv4Loopback( ulSourceIPAddress ) == pdTRUE ) ) - { - /* source OR destination is a loopback address. Make sure they BOTH are. */ - if( xBadIPv4Loopback( &( pxIPPacket->xIPHeader ) ) == pdTRUE ) - { - /* The local loopback addresses must never appear outside a host. See RFC 1122 - * section 3.2.1.3. */ - eReturn = eReleaseBuffer; - } - } - else if( memcmp( xBroadcastMACAddress.ucBytes, - pxIPPacket->xEthernetHeader.xSourceAddress.ucBytes, - sizeof( MACAddress_t ) ) == 0 ) + else if( ( memcmp( xBroadcastMACAddress.ucBytes, + pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, + sizeof( MACAddress_t ) ) == 0 ) && + ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) && ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) ) { - /* Ethernet source is a broadcast address. Drop the packet. */ + /* Ethernet address is a broadcast address, but the IP address is not a + * broadcast address. */ eReturn = eReleaseBuffer; } - else if( xIsIPv4Multicast( ulSourceIPAddress ) == pdTRUE ) + else { - /* Source is a multicast IP address. Drop the packet in conformity with RFC 1112 section 7.2. */ - eReturn = eReleaseBuffer; + /* Packet is not fragmented, destination is this device, source IP and MAC + * addresses are correct. */ } + } + else + { + /* Endpoint is down */ - /* Use ipv4_settings for filtering only after the endpoint is up, - * so that DHCP packets that are exchanged for DHCP (example, DHCP unicast offers) - * are not dropped/filtered. */ - else if( FreeRTOS_IsEndPointUp( pxEndPoint ) != pdFALSE ) + /* Check if the destination MAC address is a broadcast MAC address. */ + if( memcmp( xBroadcastMACAddress.ucBytes, + pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, + sizeof( MACAddress_t ) ) == 0 ) { - if( - /* Not destined for the assigned endpoint IPv4 address? */ - ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulIPAddress ) && - /* Also not an IPv4 broadcast address ? */ - ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) && - ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) && - /* And not an IPv4 multicast address ? */ - ( xIsIPv4Multicast( ulDestinationIPAddress ) == pdFALSE ) ) - { - /* Packet is not for this node, release it */ - eReturn = eReleaseBuffer; - } - /* Is the source address correct? */ - else if( ( ulSourceIPAddress == pxEndPoint->ipv4_settings.ulBroadcastAddress ) || - ( ulSourceIPAddress == FREERTOS_INADDR_BROADCAST ) ) - { - /* The source address cannot be broadcast address. Replying to this - * packet may cause network storms. Drop the packet. */ - eReturn = eReleaseBuffer; - } - else if( ( memcmp( xBroadcastMACAddress.ucBytes, - pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, - sizeof( MACAddress_t ) ) == 0 ) && - ( ulDestinationIPAddress != pxEndPoint->ipv4_settings.ulBroadcastAddress ) && ( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) ) + if( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) { /* Ethernet address is a broadcast address, but the IP address is not a * broadcast address. */ @@ -431,53 +439,61 @@ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * cons } else { - /* Packet is not fragmented, destination is this device, source IP and MAC - * addresses are correct. */ + /* Accept valid broadcast packet. */ } } + + /* RFC 2131: https://datatracker.ietf.org/doc/html/rfc2131#autoid-8 + * The TCP/IP software SHOULD accept and + * forward to the IP layer any IP packets delivered to the client's + * hardware address before the IP address is configured; DHCP servers + * and BOOTP relay agents may not be able to deliver DHCP messages to + * clients that cannot accept hardware unicast datagrams before the + * TCP/IP software is configured. */ + else if( ( memcmp( pxEndPoint->xMACAddress.ucBytes, + pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, + sizeof( MACAddress_t ) ) != 0 ) ) + { + /* The endpoint is not up, and the destination MAC address of the + * packet is not matching the endpoint's MAC address nor broadcast + * MAC address. Drop the packet. */ + eReturn = eReleaseBuffer; + } else { - /* Endpoint is down */ + /* Endpoint is down, but the hardware address matches. Accept the + * packet as per RFC 2131 */ + } + } - /* Check if the destination MAC address is a broadcast MAC address. */ - if( memcmp( xBroadcastMACAddress.ucBytes, - pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, - sizeof( MACAddress_t ) ) == 0 ) - { - if( ulDestinationIPAddress != FREERTOS_INADDR_BROADCAST ) - { - /* Ethernet address is a broadcast address, but the IP address is not a - * broadcast address. */ - eReturn = eReleaseBuffer; - } - else - { - /* Accept valid broadcast packet. */ - } - } + return eReturn; +} +/*-----------------------------------------------------------*/ - /* RFC 2131: https://datatracker.ietf.org/doc/html/rfc2131#autoid-8 - * The TCP/IP software SHOULD accept and - * forward to the IP layer any IP packets delivered to the client's - * hardware address before the IP address is configured; DHCP servers - * and BOOTP relay agents may not be able to deliver DHCP messages to - * clients that cannot accept hardware unicast datagrams before the - * TCP/IP software is configured. */ - else if( ( memcmp( pxEndPoint->xMACAddress.ucBytes, - pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, - sizeof( MACAddress_t ) ) != 0 ) ) - { - /* The endpoint is not up, and the destination MAC address of the - * packet is not matching the endpoint's MAC address nor broadcast - * MAC address. Drop the packet. */ - eReturn = eReleaseBuffer; - } - else - { - /* Endpoint is down, but the hardware address matches. Accept the - * packet as per RFC 2131 */ - } - } +/** + * @brief Check whether this IPv4 packet is to be allowed or to be dropped. + * + * @param[in] pxIPPacket The IP packet under consideration. + * @param[in] pxNetworkBuffer The whole network buffer. + * @param[in] uxHeaderLength The length of the header. + * + * @return Whether the packet should be processed or dropped. + */ +enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * const pxIPPacket, + const struct xNETWORK_BUFFER * const pxNetworkBuffer, + UBaseType_t uxHeaderLength ) +{ + eFrameProcessingResult_t eReturn = eProcessBuffer; + + #if ( ipconfigDRIVER_INCLUDED_RX_IP_CHECKSUM == 0 ) + const IPHeader_t * pxIPHeader = &( pxIPPacket->xIPHeader ); + #elif ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS != 0 ) + ( void ) pxIPPacket; + #endif + + #if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) + { + eReturn = eConsiderIPv4PacketForProcessing( pxIPPacket, pxNetworkBuffer->pxEndPoint, pdTRUE ); } #endif /* ipconfigETHERNET_DRIVER_FILTERS_PACKETS */ diff --git a/source/FreeRTOS_IPv6.c b/source/FreeRTOS_IPv6.c index 0eebae325..e2c996c03 100644 --- a/source/FreeRTOS_IPv6.c +++ b/source/FreeRTOS_IPv6.c @@ -291,8 +291,6 @@ BaseType_t xIsIPv6Loopback( const IPv6_Address_t * pxAddress ) return xReturn; } -#if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) - /** * @brief Check if the packet is an illegal loopback packet. * @@ -305,28 +303,26 @@ BaseType_t xIsIPv6Loopback( const IPv6_Address_t * pxAddress ) /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-89 */ /* coverity[misra_c_2012_rule_8_9_violation] */ /* coverity[single_use] */ - BaseType_t xBadIPv6Loopback( const IPHeader_IPv6_t * const pxIPv6Header ) +BaseType_t xBadIPv6Loopback( const IPHeader_IPv6_t * const pxIPv6Header ) +{ + BaseType_t xReturn = pdFALSE; + const NetworkEndPoint_t * pxEndPoint = FreeRTOS_FindEndPointOnIP_IPv6( &( pxIPv6Header->xSourceAddress ) ); + + /* Allow loopback packets from this node itself only. */ + if( pxEndPoint != NULL ) { - BaseType_t xReturn = pdFALSE; - const NetworkEndPoint_t * pxEndPoint = FreeRTOS_FindEndPointOnIP_IPv6( &( pxIPv6Header->xSourceAddress ) ); + BaseType_t x1 = ( xIsIPv6Loopback( &( pxIPv6Header->xDestinationAddress ) ) != 0 ) ? pdTRUE : pdFALSE; + BaseType_t x2 = ( xIsIPv6Loopback( &( pxIPv6Header->xSourceAddress ) ) != 0 ) ? pdTRUE : pdFALSE; - /* Allow loopback packets from this node itself only. */ - if( pxEndPoint != NULL ) + if( x1 != x2 ) { - BaseType_t x1 = ( xIsIPv6Loopback( &( pxIPv6Header->xDestinationAddress ) ) != 0 ) ? pdTRUE : pdFALSE; - BaseType_t x2 = ( xIsIPv6Loopback( &( pxIPv6Header->xSourceAddress ) ) != 0 ) ? pdTRUE : pdFALSE; - - if( x1 != x2 ) - { - /* Either source or the destination address is a loopback address. */ - xReturn = pdTRUE; - } + /* Either source or the destination address is a loopback address. */ + xReturn = pdTRUE; } - - return xReturn; } -#endif /* ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 */ + return xReturn; +} /*-----------------------------------------------------------*/ @@ -464,6 +460,77 @@ BaseType_t xCompareIPv6_Address( const IPv6_Address_t * pxLeft, /*-----------------------------------------------------------*/ +/** + * @brief Apply IPv6 admission policy shared by the IP task and driver filter. + * + * The caller must provide a complete IPv6 header. Length, checksum and transport + * validation remain the caller's responsibility. + * + * @param[in] pxIPv6Header IPv6 header to consider. + * @param[in] pxEndPoint Endpoint selected for this packet, or NULL. + * @param[in] xAllowHostLoopback pdTRUE to preserve the IP task's local loopback + * handling; pdFALSE for network driver filtering. + * @return Whether the packet should be processed or dropped. + */ +eFrameProcessingResult_t eConsiderIPv6PacketForProcessing( const IPHeader_IPv6_t * const pxIPv6Header, + const NetworkEndPoint_t * const pxEndPoint, + BaseType_t xAllowHostLoopback ) +{ + eFrameProcessingResult_t eReturn; + const IPv6_Address_t * pxDestinationIPAddress = &( pxIPv6Header->xDestinationAddress ); + const IPv6_Address_t * pxSourceIPAddress = &( pxIPv6Header->xSourceAddress ); + BaseType_t xHasUnspecifiedAddress = pdFALSE; + uint16_t ucVersionTrafficClass = pxIPv6Header->ucVersionTrafficClass; + + /* Drop if packet has unspecified IPv6 address (defined in RFC4291 - sec 2.5.2) + * either in source or destination address. */ + if( ( memcmp( pxDestinationIPAddress->ucBytes, FreeRTOS_in6addr_any.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) || + ( memcmp( pxSourceIPAddress->ucBytes, FreeRTOS_in6addr_any.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) ) + { + xHasUnspecifiedAddress = pdTRUE; + } + + /* Test if the IP-version is 6. */ + if( ( ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) != 6U ) + { + /* Can not handle, unknown or invalid header version. */ + eReturn = eReleaseBuffer; + } + else if( ( xAllowHostLoopback == pdFALSE ) && + ( ( xIsIPv6Loopback( pxSourceIPAddress ) == pdTRUE ) || + ( xIsIPv6Loopback( pxDestinationIPAddress ) == pdTRUE ) ) ) + { + /* Loopback addresses must not arrive from a physical interface. */ + eReturn = eReleaseBuffer; + } + /* Is the packet for this IP address? */ + else if( ( xHasUnspecifiedAddress == pdFALSE ) && + ( pxEndPoint != NULL ) && + ( memcmp( pxDestinationIPAddress->ucBytes, pxEndPoint->ipv6_settings.xIPAddress.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) ) + { + eReturn = eProcessBuffer; + } + /* Is it the legal multicast address? */ + else if( ( ( xHasUnspecifiedAddress == pdFALSE ) && + ( ( xAllowHostLoopback == pdFALSE ) || + ( xBadIPv6Loopback( pxIPv6Header ) == pdFALSE ) ) ) && + ( ( xIsIPv6AllowedMulticast( pxDestinationIPAddress ) != pdFALSE ) || + /* Or (during DHCP negotiation) we have no IP-address yet? */ + ( FreeRTOS_IsNetworkUp() == 0 ) ) ) + { + eReturn = eProcessBuffer; + } + else + { + /* Packet is not for this node, or the network is still not up, + * release it */ + eReturn = eReleaseBuffer; + } + + return eReturn; +} +/*-----------------------------------------------------------*/ + /** * @brief Check whether this IPv6 packet is to be allowed or to be dropped. * @@ -481,51 +548,13 @@ eFrameProcessingResult_t prvAllowIPPacketIPv6( const IPHeader_IPv6_t * const pxI #if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) { - /* In systems with a very small amount of RAM, it might be advantageous - * to have incoming messages checked earlier, by the network card driver. - * This method may decrease the usage of sparse network buffers. */ - const IPv6_Address_t * pxDestinationIPAddress = &( pxIPv6Header->xDestinationAddress ); - const IPv6_Address_t * pxSourceIPAddress = &( pxIPv6Header->xSourceAddress ); - BaseType_t xHasUnspecifiedAddress = pdFALSE; - uint16_t ucVersionTrafficClass = pxIPv6Header->ucVersionTrafficClass; - - /* Drop if packet has unspecified IPv6 address (defined in RFC4291 - sec 2.5.2) - * either in source or destination address. */ - if( ( memcmp( pxDestinationIPAddress->ucBytes, FreeRTOS_in6addr_any.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) || - ( memcmp( pxSourceIPAddress->ucBytes, FreeRTOS_in6addr_any.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) ) - { - xHasUnspecifiedAddress = pdTRUE; - } + eReturn = eConsiderIPv6PacketForProcessing( pxIPv6Header, + ( pxNetworkBuffer != NULL ) ? pxNetworkBuffer->pxEndPoint : NULL, + pdTRUE ); - /* Test if the IP-version is 6. */ - if( ( ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) != 6U ) - { - /* Can not handle, unknown or invalid header version. */ - eReturn = eReleaseBuffer; - FreeRTOS_printf( ( "prvAllowIPPacketIPv6: drop packet, invalid header version: %u\n", ( ucVersionTrafficClass & ( uint8_t ) 0xF0U ) >> 4 ) ); - } - /* Is the packet for this IP address? */ - else if( ( xHasUnspecifiedAddress == pdFALSE ) && - ( pxNetworkBuffer->pxEndPoint != NULL ) && - ( memcmp( pxDestinationIPAddress->ucBytes, pxNetworkBuffer->pxEndPoint->ipv6_settings.xIPAddress.ucBytes, sizeof( IPv6_Address_t ) ) == 0 ) ) - { - eReturn = eProcessBuffer; - } - /* Is it the legal multicast address? */ - else if( ( ( xHasUnspecifiedAddress == pdFALSE ) && - ( xBadIPv6Loopback( pxIPv6Header ) == pdFALSE ) ) && - ( ( xIsIPv6AllowedMulticast( pxDestinationIPAddress ) != pdFALSE ) || - /* Or (during DHCP negotiation) we have no IP-address yet? */ - ( FreeRTOS_IsNetworkUp() == 0 ) ) ) - { - eReturn = eProcessBuffer; - } - else + if( eReturn != eProcessBuffer ) { - /* Packet is not for this node, or the network is still not up, - * release it */ - eReturn = eReleaseBuffer; - FreeRTOS_printf( ( "prvAllowIPPacketIPv6: drop %pip (from %pip)\n", pxDestinationIPAddress->ucBytes, pxIPv6Header->xSourceAddress.ucBytes ) ); + FreeRTOS_printf( ( "prvAllowIPPacketIPv6: drop %pip (from %pip)\n", pxIPv6Header->xDestinationAddress.ucBytes, pxIPv6Header->xSourceAddress.ucBytes ) ); } } #else /* if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) */ diff --git a/source/include/FreeRTOSIPConfigDefaults.h b/source/include/FreeRTOSIPConfigDefaults.h index 2b2bfbe17..92ec01714 100644 --- a/source/include/FreeRTOSIPConfigDefaults.h +++ b/source/include/FreeRTOSIPConfigDefaults.h @@ -730,6 +730,11 @@ STATIC_ASSERT( ipconfigMAX_IP_TASK_SLEEP_TIME <= portMAX_DELAY ); * also checking the target IP address. Also when disabled, xPortHasUDPSocket() * won't be included. That means that the IP-task can access the * 'xBoundUDPSocketsList' without locking. + * + * A network driver can call eConsiderPacketForProcessing() after assigning + * the packet's matching endpoint to perform the common IP address and header + * sanity checks. The helper is available regardless of this setting; enabling + * this setting tells the IP-task that the driver has already done these checks. */ #ifndef ipconfigETHERNET_DRIVER_FILTERS_PACKETS diff --git a/source/include/FreeRTOS_IP_Private.h b/source/include/FreeRTOS_IP_Private.h index 881adfccf..c778a93a5 100644 --- a/source/include/FreeRTOS_IP_Private.h +++ b/source/include/FreeRTOS_IP_Private.h @@ -427,6 +427,14 @@ eFrameProcessingResult_t eARPProcessPacket( const NetworkBufferDescriptor_t * px */ eFrameProcessingResult_t eConsiderFrameForProcessing( const uint8_t * const pucEthernetBuffer ); +/* + * Inspect the IP addresses and other packet header fields that a filtering + * network driver is required to validate. pxEndPoint must already contain + * the endpoint selected for the packet. This helper is available independently + * of ipconfigETHERNET_DRIVER_FILTERS_PACKETS. + */ +eFrameProcessingResult_t eConsiderPacketForProcessing( const NetworkBufferDescriptor_t * const pxNetworkBuffer ); + /* * Return the checksum generated over xDataLengthBytes from pucNextData. */ diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index ebd484d72..5066b622b 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -126,6 +126,11 @@ struct xTCP_PACKET typedef struct xTCP_PACKET TCPPacket_t; +/* Shared header admission policy. See implementation for caller preconditions. */ +eFrameProcessingResult_t eConsiderIPv4PacketForProcessing( const IPPacket_t * const pxIPPacket, + const struct xNetworkEndPoint * const pxEndPoint, + BaseType_t xAllowHostLoopback ); + /* The function 'prvAllowIPPacket()' checks if a packets should be processed. */ enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * const pxIPPacket, const struct xNETWORK_BUFFER * const pxNetworkBuffer, diff --git a/source/include/FreeRTOS_IPv6.h b/source/include/FreeRTOS_IPv6.h index 5481feebd..122e6cef4 100644 --- a/source/include/FreeRTOS_IPv6.h +++ b/source/include/FreeRTOS_IPv6.h @@ -83,20 +83,23 @@ extern const struct xIPv6_Address FreeRTOS_in6addr_loopback; struct xNetworkEndPoint; struct xNetworkInterface; +/* Shared header admission policy. See implementation for caller preconditions. */ +eFrameProcessingResult_t eConsiderIPv6PacketForProcessing( const IPHeader_IPv6_t * const pxIPv6Header, + const struct xNetworkEndPoint * const pxEndPoint, + BaseType_t xAllowHostLoopback ); + /* The function 'prvAllowIPPacket()' checks if a IPv6 packets should be processed. */ eFrameProcessingResult_t prvAllowIPPacketIPv6( const IPHeader_IPv6_t * const pxIPv6Header, const NetworkBufferDescriptor_t * const pxNetworkBuffer, UBaseType_t uxHeaderLength ); -#if ( ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 ) - /* * Return pdTRUE if either source or destination is a loopback address. * A loopback IP-address may only communicate internally with another * loopback IP-address. */ - BaseType_t xBadIPv6Loopback( const IPHeader_IPv6_t * const pxIPv6Header ); -#endif /* ipconfigETHERNET_DRIVER_FILTERS_PACKETS == 0 */ +BaseType_t xBadIPv6Loopback( const IPHeader_IPv6_t * const pxIPv6Header ); + /* * Check if the address is a loopback IP-address. diff --git a/test/unit-test/FreeRTOS_IP/FreeRTOS_IP_utest.c b/test/unit-test/FreeRTOS_IP/FreeRTOS_IP_utest.c index e34fd1b3b..e1ccfc2a2 100644 --- a/test/unit-test/FreeRTOS_IP/FreeRTOS_IP_utest.c +++ b/test/unit-test/FreeRTOS_IP/FreeRTOS_IP_utest.c @@ -4457,3 +4457,33 @@ void test_prvIPNetworkUpCalls_Multicast() prvIPNetworkUpCalls_Generic( ucAddress, eIPv6_Multicast, ipHAS_IPV6 | ipHAS_METHOD | ipHAS_INTERFACE ); } + +void test_eConsiderPacketForProcessing_AvailableWithDriverFilteringDisabled( void ) +{ + NetworkBufferDescriptor_t xBuffer = { 0 }; + NetworkEndPoint_t xEndPoint = { 0 }; + IPPacket_t xIPv4Packet = { 0 }; + IPPacket_IPv6_t xIPv6Packet = { 0 }; + + TEST_ASSERT_EQUAL( ipconfigDISABLE, ipconfigETHERNET_DRIVER_FILTERS_PACKETS ); + xBuffer.pxEndPoint = &xEndPoint; + xBuffer.pucEthernetBuffer = ( uint8_t * ) &xIPv4Packet; + xBuffer.xDataLength = sizeof( xIPv4Packet ); + xIPv4Packet.xEthernetHeader.usFrameType = ipIPv4_FRAME_TYPE; + xIPv4Packet.xIPHeader.ucVersionHeaderLength = ipIPV4_VERSION_HEADER_LENGTH_MIN; + + eConsiderIPv4PacketForProcessing_ExpectAndReturn( &xIPv4Packet, &xEndPoint, pdFALSE, eProcessBuffer ); + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xBuffer ) ); + eConsiderIPv4PacketForProcessing_ExpectAndReturn( &xIPv4Packet, &xEndPoint, pdFALSE, eReleaseBuffer ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xBuffer ) ); + + xEndPoint.bits.bIPv6 = pdTRUE_UNSIGNED; + xBuffer.pucEthernetBuffer = ( uint8_t * ) &xIPv6Packet; + xBuffer.xDataLength = sizeof( xIPv6Packet ); + xIPv6Packet.xEthernetHeader.usFrameType = ipIPv6_FRAME_TYPE; + + eConsiderIPv6PacketForProcessing_ExpectAndReturn( &xIPv6Packet.xIPHeader, &xEndPoint, pdFALSE, eProcessBuffer ); + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xBuffer ) ); + eConsiderIPv6PacketForProcessing_ExpectAndReturn( &xIPv6Packet.xIPHeader, &xEndPoint, pdFALSE, eReleaseBuffer ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xBuffer ) ); +} diff --git a/test/unit-test/FreeRTOS_IP_DiffConfig3/FreeRTOS_IP_DiffConfig3_utest.c b/test/unit-test/FreeRTOS_IP_DiffConfig3/FreeRTOS_IP_DiffConfig3_utest.c index aba50ccd0..1e20c6387 100644 --- a/test/unit-test/FreeRTOS_IP_DiffConfig3/FreeRTOS_IP_DiffConfig3_utest.c +++ b/test/unit-test/FreeRTOS_IP_DiffConfig3/FreeRTOS_IP_DiffConfig3_utest.c @@ -48,8 +48,11 @@ #include "mock_FreeRTOS_DHCPv6.h" #include "mock_NetworkBufferManagement.h" #include "mock_FreeRTOS_Routing.h" +#include "mock_FreeRTOS_IPv4_Private.h" +#include "mock_FreeRTOS_IPv6.h" #include "FreeRTOS_IP.h" +#include "FreeRTOS_IP_Private.h" /*#include "FreeRTOS_IP_stubs.c" */ #include "catch_assert.h" @@ -84,6 +87,55 @@ void tearDown( void ) { } +static void prvPrepareIPv4Packet( NetworkBufferDescriptor_t * pxNetworkBuffer, + NetworkEndPoint_t * pxEndPoint, + IPPacket_t * pxIPPacket ) +{ + ( void ) memset( pxNetworkBuffer, 0, sizeof( *pxNetworkBuffer ) ); + ( void ) memset( pxEndPoint, 0, sizeof( *pxEndPoint ) ); + ( void ) memset( pxIPPacket, 0, sizeof( *pxIPPacket ) ); + + pxNetworkBuffer->pucEthernetBuffer = ( uint8_t * ) pxIPPacket; + pxNetworkBuffer->xDataLength = sizeof( *pxIPPacket ); + pxNetworkBuffer->pxEndPoint = pxEndPoint; + + pxEndPoint->bits.bEndPointUp = pdTRUE_UNSIGNED; + pxEndPoint->ipv4_settings.ulIPAddress = 0x01020304U; + pxEndPoint->ipv4_settings.ulBroadcastAddress = 0x010203FFU; + ( void ) memset( pxEndPoint->xMACAddress.ucBytes, 0x11, sizeof( MACAddress_t ) ); + + pxIPPacket->xEthernetHeader.usFrameType = ipIPv4_FRAME_TYPE; + ( void ) memcpy( pxIPPacket->xEthernetHeader.xDestinationAddress.ucBytes, + pxEndPoint->xMACAddress.ucBytes, + sizeof( MACAddress_t ) ); + ( void ) memset( pxIPPacket->xEthernetHeader.xSourceAddress.ucBytes, 0x22, sizeof( MACAddress_t ) ); + pxIPPacket->xIPHeader.ucVersionHeaderLength = ipIPV4_VERSION_HEADER_LENGTH_MIN; + pxIPPacket->xIPHeader.ulDestinationIPAddress = pxEndPoint->ipv4_settings.ulIPAddress; + pxIPPacket->xIPHeader.ulSourceIPAddress = 0x05060708U; +} + +static void prvPrepareIPv6Packet( NetworkBufferDescriptor_t * pxNetworkBuffer, + NetworkEndPoint_t * pxEndPoint, + IPPacket_IPv6_t * pxIPPacket ) +{ + ( void ) memset( pxNetworkBuffer, 0, sizeof( *pxNetworkBuffer ) ); + ( void ) memset( pxEndPoint, 0, sizeof( *pxEndPoint ) ); + ( void ) memset( pxIPPacket, 0, sizeof( *pxIPPacket ) ); + + pxNetworkBuffer->pucEthernetBuffer = ( uint8_t * ) pxIPPacket; + pxNetworkBuffer->xDataLength = sizeof( *pxIPPacket ); + pxNetworkBuffer->pxEndPoint = pxEndPoint; + + pxEndPoint->bits.bIPv6 = pdTRUE_UNSIGNED; + pxEndPoint->bits.bEndPointUp = pdTRUE_UNSIGNED; + pxEndPoint->ipv6_settings.xIPAddress.ucBytes[ ipSIZE_OF_IPv6_ADDRESS - 1U ] = 2U; + + pxIPPacket->xEthernetHeader.usFrameType = ipIPv6_FRAME_TYPE; + pxIPPacket->xIPHeader.ucVersionTrafficClass = 0x60U; + pxIPPacket->xIPHeader.xDestinationAddress = pxEndPoint->ipv6_settings.xIPAddress; + pxIPPacket->xIPHeader.xSourceAddress.ucBytes[ ipSIZE_OF_IPv6_ADDRESS - 1U ] = 1U; +} + /* ======================== Stub Callback Functions ========================= */ eFrameProcessingResult_t eApplicationProcessCustomFrameHook( NetworkBufferDescriptor_t * const pxNetworkBuffer ) @@ -246,3 +298,118 @@ void test_prvProcessEthernetPacket_IPv4FrameType_CheckFrameFail( void ) prvProcessEthernetPacket( pxNetworkBuffer ); } + +/** + * @brief Invalid descriptors and truncated Ethernet headers must be rejected. + */ +void test_eConsiderPacketForProcessing_InvalidDescriptor( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer = { 0 }; + NetworkEndPoint_t xEndPoint = { 0 }; + uint8_t ucEthernetBuffer[ sizeof( EthernetHeader_t ) ] = { 0 }; + + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( NULL ) ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + xNetworkBuffer.pucEthernetBuffer = ucEthernetBuffer; + xNetworkBuffer.xDataLength = sizeof( ucEthernetBuffer ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + xNetworkBuffer.pxEndPoint = &xEndPoint; + xNetworkBuffer.xDataLength--; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} + +/** + * @brief ARP and enabled custom Ethernet frames pass the common packet filter. + */ +void test_eConsiderPacketForProcessing_ARPAndCustomFrames( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer = { 0 }; + NetworkEndPoint_t xEndPoint = { 0 }; + EthernetHeader_t xEthernetHeader = { 0 }; + + xNetworkBuffer.pucEthernetBuffer = ( uint8_t * ) &xEthernetHeader; + xNetworkBuffer.xDataLength = sizeof( xEthernetHeader ); + xNetworkBuffer.pxEndPoint = &xEndPoint; + + xEthernetHeader.usFrameType = ipARP_FRAME_TYPE; + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + xEthernetHeader.usFrameType = 0xFFFFU; + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} + +/** + * @brief Validate framing before dispatching IPv4 admission policy. + */ +void test_eConsiderPacketForProcessing_IPv4HeaderValidation( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer; + NetworkEndPoint_t xEndPoint; + IPPacket_t xIPPacket; + + prvPrepareIPv4Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + xEndPoint.bits.bIPv6 = pdTRUE_UNSIGNED; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + prvPrepareIPv4Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + xNetworkBuffer.xDataLength = sizeof( xIPPacket ) - 1U; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + prvPrepareIPv4Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + xIPPacket.xIPHeader.ucVersionHeaderLength = ipIPV4_VERSION_HEADER_LENGTH_MAX; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} + +/** + * @brief Delegate IPv4 policy and return its decision without host exceptions. + */ +void test_eConsiderPacketForProcessing_IPv4Admission( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer; + NetworkEndPoint_t xEndPoint; + IPPacket_t xIPPacket; + + prvPrepareIPv4Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + eConsiderIPv4PacketForProcessing_ExpectAndReturn( &xIPPacket, &xEndPoint, pdFALSE, eProcessBuffer ); + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + eConsiderIPv4PacketForProcessing_ExpectAndReturn( &xIPPacket, &xEndPoint, pdFALSE, eReleaseBuffer ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} + +/** + * @brief Validate framing before dispatching IPv6 admission policy. + */ +void test_eConsiderPacketForProcessing_IPv6HeaderValidation( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer; + NetworkEndPoint_t xEndPoint; + IPPacket_IPv6_t xIPPacket; + + prvPrepareIPv6Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + xEndPoint.bits.bIPv6 = pdFALSE_UNSIGNED; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + prvPrepareIPv6Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + xNetworkBuffer.xDataLength = sizeof( xIPPacket ) - 1U; + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} + +/** + * @brief Delegate IPv6 policy and return its decision without host exceptions. + */ +void test_eConsiderPacketForProcessing_IPv6Admission( void ) +{ + NetworkBufferDescriptor_t xNetworkBuffer; + NetworkEndPoint_t xEndPoint; + IPPacket_IPv6_t xIPPacket; + + prvPrepareIPv6Packet( &xNetworkBuffer, &xEndPoint, &xIPPacket ); + eConsiderIPv6PacketForProcessing_ExpectAndReturn( &xIPPacket.xIPHeader, &xEndPoint, pdFALSE, eProcessBuffer ); + TEST_ASSERT_EQUAL( eProcessBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); + + eConsiderIPv6PacketForProcessing_ExpectAndReturn( &xIPPacket.xIPHeader, &xEndPoint, pdFALSE, eReleaseBuffer ); + TEST_ASSERT_EQUAL( eReleaseBuffer, eConsiderPacketForProcessing( &xNetworkBuffer ) ); +} From 3225c4929e9a5645932af2a130418a45053db0f7 Mon Sep 17 00:00:00 2001 From: Holden Ramsey Date: Tue, 8 Sep 2026 19:39:53 -0400 Subject: [PATCH 2/2] refactor: use common packet filtering in STM32 interface Filter completed RX descriptors after assigning their matching endpoint. Replace placeholder protocol checks, validate fixed header lengths before endpoint lookup, and avoid empty events when all received packets are rejected. --- .../NetworkInterface/STM32/NetworkInterface.c | 143 ++++++------------ 1 file changed, 44 insertions(+), 99 deletions(-) diff --git a/source/portable/NetworkInterface/STM32/NetworkInterface.c b/source/portable/NetworkInterface/STM32/NetworkInterface.c index 39b11d18b..bfa93737c 100644 --- a/source/portable/NetworkInterface/STM32/NetworkInterface.c +++ b/source/portable/NetworkInterface/STM32/NetworkInterface.c @@ -49,9 +49,6 @@ #include "FreeRTOS_ND.h" #endif #include "FreeRTOS_Routing.h" -#if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) - #include "FreeRTOS_Sockets.h" -#endif #include "NetworkBufferManagement.h" #include "NetworkInterface.h" #include "phyHandling.h" @@ -161,12 +158,9 @@ #warning "Consider enabling ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES for NetworkInterface" #endif -/* TODO: There should be a universal check for use in network interfaces, similar to eConsiderFrameForProcessing. - * So, don't use this macro, and filter anyways in the mean time. */ - -/* #if ipconfigIS_DISABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) - #warning "Consider enabling ipconfigETHERNET_DRIVER_FILTERS_PACKETS for NetworkInterface" - #endif */ + #if ipconfigIS_DISABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) + #warning "Consider enabling ipconfigETHERNET_DRIVER_FILTERS_PACKETS for NetworkInterface" + #endif #if ipconfigIS_DISABLED( ipconfigUSE_LINKED_RX_MESSAGES ) #warning "Consider enabling ipconfigUSE_LINKED_RX_MESSAGES for NetworkInterface" @@ -218,32 +212,11 @@ #undef ETH_CTRLPACKETS_BLOCK_ALL #define ETH_CTRLPACKETS_BLOCK_ALL ETH_MACFFR_PCF_BlockAll - #undef ETH_IP_HEADER_IPV4 - #define ETH_IP_HEADER_IPV4 ETH_DMAPTPRXDESC_IPV4PR - - #undef ETH_IP_HEADER_IPV6 - #define ETH_IP_HEADER_IPV6 ETH_DMAPTPRXDESC_IPV6PR - - #undef ETH_IP_PAYLOAD_UNKNOWN - #define ETH_IP_PAYLOAD_UNKNOWN 0x0U - - #undef ETH_IP_PAYLOAD_UDP - #define ETH_IP_PAYLOAD_UDP ETH_DMAPTPRXDESC_IPPT_UDP - - #undef ETH_IP_PAYLOAD_TCP - #define ETH_IP_PAYLOAD_TCP ETH_DMAPTPRXDESC_IPPT_TCP - - #undef ETH_IP_PAYLOAD_ICMPN - #define ETH_IP_PAYLOAD_ICMPN ETH_DMAPTPRXDESC_IPPT_ICMP - #elif defined( niEMAC_STM32HX ) #undef ETH_DMA_TX_BUFFER_UNAVAILABLE_FLAG #define ETH_DMA_TX_BUFFER_UNAVAILABLE_FLAG ETH_DMACSR_TBU - #undef ETH_IP_PAYLOAD_IGMP - #define ETH_IP_PAYLOAD_IGMP 0x4U - #endif /* if defined( niEMAC_STM32FX ) */ #define ETH_IP_PAYLOAD_MASK 0x7U @@ -778,6 +751,18 @@ static BaseType_t prvNetworkInterfaceInput( ETH_HandleTypeDef * pxEthHandle, pxCurDescriptor->pxInterface = pxInterface; pxCurDescriptor->pxEndPoint = FreeRTOS_MatchingEndpoint( pxCurDescriptor->pxInterface, pxCurDescriptor->pucEthernetBuffer ); + + #if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) + /* The completed descriptor now has its actual frame length and + * matching endpoint, as required by the common packet filter. */ + if( eConsiderPacketForProcessing( pxCurDescriptor ) != eProcessBuffer ) + { + iptraceETHERNET_RX_EVENT_LOST(); + prvReleaseNetworkBufferDescriptor( pxCurDescriptor ); + continue; + } + #endif + #if ipconfigIS_ENABLED( ipconfigUSE_LINKED_RX_MESSAGES ) if( pxStartDescriptor == NULL ) { @@ -798,7 +783,10 @@ static BaseType_t prvNetworkInterfaceInput( ETH_HandleTypeDef * pxEthHandle, if( uxCount > 0 ) { #if ipconfigIS_ENABLED( ipconfigUSE_LINKED_RX_MESSAGES ) - prvSendRxEvent( pxStartDescriptor ); + if( pxStartDescriptor != NULL ) + { + prvSendRxEvent( pxStartDescriptor ); + } #endif xResult = pdTRUE; } @@ -1681,95 +1669,52 @@ static BaseType_t prvAcceptPacket( const NetworkBufferDescriptor_t * const pxDes break; } - ETH_HandleTypeDef * pxEthHandle = &xEthHandle; - uint32_t ulErrorCode = 0; - ( void ) HAL_ETH_GetRxDataErrorCode( pxEthHandle, &ulErrorCode ); - - if( ulErrorCode != 0 ) + if( ( pxDescriptor->pucEthernetBuffer == NULL ) || ( usLength < sizeof( EthernetHeader_t ) ) ) { iptraceETHERNET_RX_EVENT_LOST(); - FreeRTOS_debug_printf( ( "prvAcceptPacket: Rx Data Error\n" ) ); break; } - #if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES ) - if( eConsiderFrameForProcessing( pxDescriptor->pucEthernetBuffer ) != eProcessBuffer ) + /* Endpoint matching reads protocol addresses before the common packet + * filter runs, so require each protocol's fixed header first. */ + const EthernetHeader_t * const pxEthernetHeader = ( const EthernetHeader_t * ) pxDescriptor->pucEthernetBuffer; + + #if ipconfigIS_ENABLED( ipconfigUSE_IPv4 ) + if( ( ( pxEthernetHeader->usFrameType == ipIPv4_FRAME_TYPE ) && ( usLength < sizeof( IPPacket_t ) ) ) || + ( ( pxEthernetHeader->usFrameType == ipARP_FRAME_TYPE ) && ( usLength < sizeof( ARPPacket_t ) ) ) ) { iptraceETHERNET_RX_EVENT_LOST(); - FreeRTOS_debug_printf( ( "prvAcceptPacket: Frame discarded\n" ) ); break; } #endif - #if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) - { - const ETH_DMADescTypeDef * const pxRxDesc = ( const ETH_DMADescTypeDef * const ) pxEthHandle->RxDescList.RxDesc[ pxEthHandle->RxDescList.RxDescIdx ]; - uint32_t ulRxDesc; - #ifdef niEMAC_STM32HX - ulRxDesc = pxRxDesc->DESC1; - #elif defined( niEMAC_STM32FX ) - ulRxDesc = pxRxDesc->DESC4; - #endif - - if( ( ulRxDesc & ETH_IP_HEADER_IPV4 ) != 0 ) - { - /* Should be impossible if hardware filtering is implemented correctly */ - configASSERT( ipconfigIS_ENABLED( ipconfigUSE_IPv4 ) ); - #if ipconfigIS_ENABLED( ipconfigUSE_IPv4 ) - /* prvAllowIPPacketIPv4(); */ - #endif - } - else if( ( ulRxDesc & ETH_IP_HEADER_IPV6 ) != 0 ) - { - /* Should be impossible if hardware filtering is implemented correctly */ - configASSERT( ipconfigIS_ENABLED( ipconfigUSE_IPv6 ) ); - #if ipconfigIS_ENABLED( ipconfigUSE_IPv6 ) - /* prvAllowIPPacketIPv6(); */ - #endif - } - - if( ( ulRxDesc & ETH_IP_PAYLOAD_MASK ) == ETH_IP_PAYLOAD_UNKNOWN ) + #if ipconfigIS_ENABLED( ipconfigUSE_IPv6 ) + if( ( pxEthernetHeader->usFrameType == ipIPv6_FRAME_TYPE ) && ( usLength < sizeof( IPPacket_IPv6_t ) ) ) { iptraceETHERNET_RX_EVENT_LOST(); break; } - else if( ( ulRxDesc & ETH_IP_PAYLOAD_MASK ) == ETH_IP_PAYLOAD_UDP ) - { - /* prvProcessUDPPacket(); */ - } - else if( ( ulRxDesc & ETH_IP_PAYLOAD_MASK ) == ETH_IP_PAYLOAD_TCP ) - { - /* Should be impossible if hardware filtering is implemented correctly */ - configASSERT( ipconfigIS_ENABLED( ipconfigUSE_TCP ) ); - #if ipconfigIS_ENABLED( ipconfigUSE_TCP ) - /* xProcessReceivedTCPPacket() */ - #endif - } - else if( ( ulRxDesc & ETH_IP_PAYLOAD_MASK ) == ETH_IP_PAYLOAD_ICMPN ) - { - #if ipconfigIS_DISABLED( ipconfigREPLY_TO_INCOMING_PINGS ) && ipconfigIS_DISABLED( ipconfigSUPPORT_OUTGOING_PINGS ) - iptraceETHERNET_RX_EVENT_LOST(); - break; - #else - /* ProcessICMPPacket(); */ - #endif - } + #endif - #ifdef niEMAC_STM32HX - else if( ( ulRxDesc & ETH_IP_PAYLOAD_MASK ) == ETH_IP_PAYLOAD_IGMP ) - { - } - #endif + ETH_HandleTypeDef * pxEthHandle = &xEthHandle; + uint32_t ulErrorCode = 0; + ( void ) HAL_ETH_GetRxDataErrorCode( pxEthHandle, &ulErrorCode ); - /* TODO: Create a eConsiderPacketForProcessing */ - if( eConsiderPacketForProcessing( pxDescriptor->pucEthernetBuffer ) != eProcessBuffer ) + if( ulErrorCode != 0 ) + { + iptraceETHERNET_RX_EVENT_LOST(); + FreeRTOS_debug_printf( ( "prvAcceptPacket: Rx Data Error\n" ) ); + break; + } + + #if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES ) + if( eConsiderFrameForProcessing( pxDescriptor->pucEthernetBuffer ) != eProcessBuffer ) { iptraceETHERNET_RX_EVENT_LOST(); - FreeRTOS_debug_printf( ( "prvAcceptPacket: Packet discarded\n" ) ); + FreeRTOS_debug_printf( ( "prvAcceptPacket: Frame discarded\n" ) ); break; } - } - #endif /* if ipconfigIS_ENABLED( ipconfigETHERNET_DRIVER_FILTERS_PACKETS ) */ + #endif xResult = pdTRUE; } while( pdFALSE );