From 4693a12a12c0d003a411e5a88dd612e6b97799b2 Mon Sep 17 00:00:00 2001 From: Emil Popov Date: Fri, 28 Apr 2023 09:19:44 -0400 Subject: [PATCH 1/9] Removes the storing of the IP-Type byte for IPv6 frames because it never is and never should be used for these frames. Defines ipUDP_PAYLOAD_IP_TYPE_OFFSET as an offset dependent on the IPv6 and UDP headers. Calculates ipIP_TYPE_OFFSET automatically based on the sizes it depends on instead of using a hardcoded number. Removes the definitions of ipIP_TYPE_OFFSET and ipUDP_PAYLOAD_IP_TYPE_OFFSET from FreeRTOS_IPv6_Private.h because they are already defined in FreeRTOS_IPv4_Private.h Adds an assert to ensure that storing of the IP-Type for IPv4 frames does not result in overwriting the ethernet header which would be the case if ipIP_TYPE_OFFSET somehow became negative or zero. --- source/FreeRTOS_IP.c | 5 ----- source/FreeRTOS_IP_Utils.c | 4 ++++ source/include/FreeRTOS_IPv4_Private.h | 23 ++++++++++++++++++----- source/include/FreeRTOS_IPv6_Private.h | 6 ------ 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/source/FreeRTOS_IP.c b/source/FreeRTOS_IP.c index 5266bbf686..2dffc1922b 100644 --- a/source/FreeRTOS_IP.c +++ b/source/FreeRTOS_IP.c @@ -1823,11 +1823,6 @@ static eFrameProcessingResult_t prvProcessIPPacket( const IPPacket_t * pxIPPacke /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */ /* coverity[misra_c_2012_rule_11_3_violation] */ eReturn = prvAllowIPPacketIPv6( ( ( const IPHeader_IPv6_t * ) &( pxIPPacket->xIPHeader ) ), pxNetworkBuffer, uxHeaderLength ); - - /* The IP-header type is copied to a location 6 bytes before the messages - * starts. It might be needed later on when a UDP-payload buffer is being - * used. */ - pxNetworkBuffer->pucEthernetBuffer[ 0 - ( BaseType_t ) ipIP_TYPE_OFFSET ] = pxIPHeader_IPv6->ucVersionTrafficClass; } break; #endif /* ( ipconfigUSE_IPv6 != 0 ) */ diff --git a/source/FreeRTOS_IP_Utils.c b/source/FreeRTOS_IP_Utils.c index 767ccdeaaa..a51e988581 100644 --- a/source/FreeRTOS_IP_Utils.c +++ b/source/FreeRTOS_IP_Utils.c @@ -996,6 +996,10 @@ void vPreCheckConfigs( void ) } #endif /* LCOV_EXCL_BR_STOP */ + + /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) + and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ + configASSERT( ipIP_TYPE_OFFSET > 0 ); } #endif /* if ( configASSERT_DEFINED == 1 ) */ } diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index eb83b5a479..9393580adf 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -45,11 +45,24 @@ /* The offset into a UDP packet at which the UDP data (payload) starts. */ #define ipUDP_PAYLOAD_OFFSET_IPv4 ( sizeof( UDPPacket_t ) ) /* The value of 'ipUDP_PAYLOAD_IP_TYPE_OFFSET' is 42 + 6 = 48 bytes. */ -/* __HT_ just to get it compiled. */ -#if !defined( ipIP_TYPE_OFFSET ) - #define ipIP_TYPE_OFFSET ( 6U ) -#endif -#define ipUDP_PAYLOAD_IP_TYPE_OFFSET ( sizeof( UDPPacket_t ) + ipIP_TYPE_OFFSET ) +#define ipUDP_PAYLOAD_IP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) + +/* ipIP_TYPE_OFFSET is involved in some sorcery. pxUDPPayloadBuffer_to_NetworkBuffer() must be able to convert +a payload pointer ( like for example a pointer to the DNS payload of a packet ) back to a NetworkBufferDescriptor_t. +This must work for both IPv4 and IPv6 packets. The pointer conversion is done by subtracting a constant from the payload pointer. +For IPv6, this magic number is ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which equals 48 bytes. +If however we use that same constant for an IPv4 packet, we end up somewhere in front of the Ethernet header. +In order to accommodate that, the Ethernet frame buffer gets allocated a bit larger than needed. +For IPv4 frames, prvProcessIPPacket() stores the version header field at a negative offset, a few bytes before the start +of the Ethernet header. That IPv4 version field MUST be stored the same distance from the payload as in IPv6. Make sure that: +ipUDP_PAYLOAD_IP_TYPE_OFFSET == ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which expands to: +( sizeof( UDPPacket_t ) + ipIP_TYPE_OFFSET ) != ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which means that: +ipIP_TYPE_OFFSET must be equal to: sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) +In most situations, ipIP_TYPE_OFFSET will end up being equal to 6. If the Ethernet header is enlarged to include VLAN +tag support, ipIP_TYPE_OFFSET will shrink to 2. With the current design, the Ethernet header cannot be expanded to contain +more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0. + */ +#define ipIP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - sizeof( UDPPacket_t ) ) #include "pack_struct_start.h" struct xIP_HEADER diff --git a/source/include/FreeRTOS_IPv6_Private.h b/source/include/FreeRTOS_IPv6_Private.h index 6851fe8a40..669a364780 100644 --- a/source/include/FreeRTOS_IPv6_Private.h +++ b/source/include/FreeRTOS_IPv6_Private.h @@ -55,10 +55,6 @@ #undef TCP_PACKET_SIZE #define TCP_PACKET_SIZE ( sizeof( TCPPacket_IPv6_t ) ) -/* The offset from the UDP payload where the IP type will be stored. - * For IPv4 packets, this it located 6 bytes before pucEthernetBuffer. - * For IPv6 packets, this it located in the usual 'ucVersionTrafficClass'. */ -#define ipIP_TYPE_OFFSET ( 6U ) /* The offset into an IP packet into which the IP data (payload) starts. */ #define ipIPv6_PAYLOAD_OFFSET ( sizeof( IPPacket_IPv6_t ) ) /* MISRA Ref 20.5.1 [Use of undef] */ @@ -69,8 +65,6 @@ #define ipMAX_UDP_PAYLOAD_LENGTH ( ( ipconfigNETWORK_MTU - ipSIZE_OF_IPv6_HEADER ) - ipSIZE_OF_UDP_HEADER ) /* The offset into a UDP packet at which the UDP data (payload) starts. */ #define ipUDP_PAYLOAD_OFFSET_IPv6 ( sizeof( UDPPacket_IPv6_t ) ) -/* The value of 'ipUDP_PAYLOAD_IP_TYPE_OFFSET' is 42 + 6 = 48 bytes. */ -#define ipUDP_PAYLOAD_IPv6_TYPE_OFFSET ( sizeof( UDPPacket_IPv6_t ) + ipIP_TYPE_OFFSET ) #if ( ipconfigBYTE_ORDER == pdFREERTOS_LITTLE_ENDIAN ) From 971d1b043f12e07074c6b017c7f40270e466c497 Mon Sep 17 00:00:00 2001 From: Kody Stribrny Date: Thu, 29 Jun 2023 10:04:53 -0700 Subject: [PATCH 2/9] Uncrustify Formatting --- source/FreeRTOS_IP_Utils.c | 8 +++--- source/include/FreeRTOS_IPv4_Private.h | 36 +++++++++++++------------- source/include/FreeRTOS_IPv6_Private.h | 4 +-- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/source/FreeRTOS_IP_Utils.c b/source/FreeRTOS_IP_Utils.c index a51e988581..6b6be1b727 100644 --- a/source/FreeRTOS_IP_Utils.c +++ b/source/FreeRTOS_IP_Utils.c @@ -996,10 +996,10 @@ void vPreCheckConfigs( void ) } #endif /* LCOV_EXCL_BR_STOP */ - - /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) - and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ - configASSERT( ipIP_TYPE_OFFSET > 0 ); + + /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) + * and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ + configASSERT( ipIP_TYPE_OFFSET > 0 ); } #endif /* if ( configASSERT_DEFINED == 1 ) */ } diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index 9393580adf..5339c3ab8a 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -36,33 +36,33 @@ #include "FreeRTOS_IP_Private.h" /* The maximum UDP payload length. */ -#define ipMAX_UDP_PAYLOAD_LENGTH ( ( ipconfigNETWORK_MTU - ipSIZE_OF_IPv4_HEADER ) - ipSIZE_OF_UDP_HEADER ) +#define ipMAX_UDP_PAYLOAD_LENGTH ( ( ipconfigNETWORK_MTU - ipSIZE_OF_IPv4_HEADER ) - ipSIZE_OF_UDP_HEADER ) -#define TCP_PACKET_SIZE ( sizeof( TCPPacket_t ) ) +#define TCP_PACKET_SIZE ( sizeof( TCPPacket_t ) ) /* The offset into an IP packet into which the IP data (payload) starts. */ -#define ipIP_PAYLOAD_OFFSET ( sizeof( IPPacket_t ) ) +#define ipIP_PAYLOAD_OFFSET ( sizeof( IPPacket_t ) ) /* The offset into a UDP packet at which the UDP data (payload) starts. */ -#define ipUDP_PAYLOAD_OFFSET_IPv4 ( sizeof( UDPPacket_t ) ) +#define ipUDP_PAYLOAD_OFFSET_IPv4 ( sizeof( UDPPacket_t ) ) /* The value of 'ipUDP_PAYLOAD_IP_TYPE_OFFSET' is 42 + 6 = 48 bytes. */ #define ipUDP_PAYLOAD_IP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) /* ipIP_TYPE_OFFSET is involved in some sorcery. pxUDPPayloadBuffer_to_NetworkBuffer() must be able to convert -a payload pointer ( like for example a pointer to the DNS payload of a packet ) back to a NetworkBufferDescriptor_t. -This must work for both IPv4 and IPv6 packets. The pointer conversion is done by subtracting a constant from the payload pointer. -For IPv6, this magic number is ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which equals 48 bytes. -If however we use that same constant for an IPv4 packet, we end up somewhere in front of the Ethernet header. -In order to accommodate that, the Ethernet frame buffer gets allocated a bit larger than needed. -For IPv4 frames, prvProcessIPPacket() stores the version header field at a negative offset, a few bytes before the start -of the Ethernet header. That IPv4 version field MUST be stored the same distance from the payload as in IPv6. Make sure that: -ipUDP_PAYLOAD_IP_TYPE_OFFSET == ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which expands to: -( sizeof( UDPPacket_t ) + ipIP_TYPE_OFFSET ) != ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which means that: -ipIP_TYPE_OFFSET must be equal to: sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) -In most situations, ipIP_TYPE_OFFSET will end up being equal to 6. If the Ethernet header is enlarged to include VLAN -tag support, ipIP_TYPE_OFFSET will shrink to 2. With the current design, the Ethernet header cannot be expanded to contain -more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0. + * a payload pointer ( like for example a pointer to the DNS payload of a packet ) back to a NetworkBufferDescriptor_t. + * This must work for both IPv4 and IPv6 packets. The pointer conversion is done by subtracting a constant from the payload pointer. + * For IPv6, this magic number is ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which equals 48 bytes. + * If however we use that same constant for an IPv4 packet, we end up somewhere in front of the Ethernet header. + * In order to accommodate that, the Ethernet frame buffer gets allocated a bit larger than needed. + * For IPv4 frames, prvProcessIPPacket() stores the version header field at a negative offset, a few bytes before the start + * of the Ethernet header. That IPv4 version field MUST be stored the same distance from the payload as in IPv6. Make sure that: + * ipUDP_PAYLOAD_IP_TYPE_OFFSET == ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which expands to: + * ( sizeof( UDPPacket_t ) + ipIP_TYPE_OFFSET ) != ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which means that: + * ipIP_TYPE_OFFSET must be equal to: sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) + * In most situations, ipIP_TYPE_OFFSET will end up being equal to 6. If the Ethernet header is enlarged to include VLAN + * tag support, ipIP_TYPE_OFFSET will shrink to 2. With the current design, the Ethernet header cannot be expanded to contain + * more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0. */ -#define ipIP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - sizeof( UDPPacket_t ) ) +#define ipIP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - sizeof( UDPPacket_t ) ) #include "pack_struct_start.h" struct xIP_HEADER diff --git a/source/include/FreeRTOS_IPv6_Private.h b/source/include/FreeRTOS_IPv6_Private.h index 669a364780..16802aff5d 100644 --- a/source/include/FreeRTOS_IPv6_Private.h +++ b/source/include/FreeRTOS_IPv6_Private.h @@ -62,9 +62,9 @@ /* coverity[misra_c_2012_rule_20_5_violation] */ /* The maximum UDP payload length. */ #undef ipMAX_UDP_PAYLOAD_LENGTH -#define ipMAX_UDP_PAYLOAD_LENGTH ( ( ipconfigNETWORK_MTU - ipSIZE_OF_IPv6_HEADER ) - ipSIZE_OF_UDP_HEADER ) +#define ipMAX_UDP_PAYLOAD_LENGTH ( ( ipconfigNETWORK_MTU - ipSIZE_OF_IPv6_HEADER ) - ipSIZE_OF_UDP_HEADER ) /* The offset into a UDP packet at which the UDP data (payload) starts. */ -#define ipUDP_PAYLOAD_OFFSET_IPv6 ( sizeof( UDPPacket_IPv6_t ) ) +#define ipUDP_PAYLOAD_OFFSET_IPv6 ( sizeof( UDPPacket_IPv6_t ) ) #if ( ipconfigBYTE_ORDER == pdFREERTOS_LITTLE_ENDIAN ) From 0a18c295908f5521dcb92fdb9ff611e9dbb29370 Mon Sep 17 00:00:00 2001 From: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Thu, 29 Jun 2023 10:54:03 -0700 Subject: [PATCH 3/9] Modify formatting to showcase use of workflow @evpopov I am modifying just enough to make uncrustify unhappy. --- source/FreeRTOS_IP_Utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FreeRTOS_IP_Utils.c b/source/FreeRTOS_IP_Utils.c index 6b6be1b727..df77528e57 100644 --- a/source/FreeRTOS_IP_Utils.c +++ b/source/FreeRTOS_IP_Utils.c @@ -999,7 +999,7 @@ void vPreCheckConfigs( void ) /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) * and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ - configASSERT( ipIP_TYPE_OFFSET > 0 ); + configASSERT( ipIP_TYPE_OFFSET > 0 ); } #endif /* if ( configASSERT_DEFINED == 1 ) */ } From 4ede1bf283bcc477261e68a40d1c411e99f6b3a6 Mon Sep 17 00:00:00 2001 From: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> Date: Thu, 29 Jun 2023 10:57:04 -0700 Subject: [PATCH 4/9] Kick the GHA workflow to trigger --- source/FreeRTOS_IP_Utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FreeRTOS_IP_Utils.c b/source/FreeRTOS_IP_Utils.c index df77528e57..0ae337c5fc 100644 --- a/source/FreeRTOS_IP_Utils.c +++ b/source/FreeRTOS_IP_Utils.c @@ -999,7 +999,7 @@ void vPreCheckConfigs( void ) /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) * and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ - configASSERT( ipIP_TYPE_OFFSET > 0 ); + configASSERT( ipIP_TYPE_OFFSET > 0 ); } #endif /* if ( configASSERT_DEFINED == 1 ) */ } From c7d562783522a4a37ff0863279af8ae52215b7be Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Thu, 29 Jun 2023 18:16:41 +0000 Subject: [PATCH 5/9] Uncrustify: triggered by comment. --- source/FreeRTOS_IP_Utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/FreeRTOS_IP_Utils.c b/source/FreeRTOS_IP_Utils.c index 0ae337c5fc..6b6be1b727 100644 --- a/source/FreeRTOS_IP_Utils.c +++ b/source/FreeRTOS_IP_Utils.c @@ -999,7 +999,7 @@ void vPreCheckConfigs( void ) /* ipIP_TYPE_OFFSET = ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) ) * and it MUST be > 0, otherwise storing the IPv4 version byte will overwrite the Ethernet header. */ - configASSERT( ipIP_TYPE_OFFSET > 0 ); + configASSERT( ipIP_TYPE_OFFSET > 0 ); } #endif /* if ( configASSERT_DEFINED == 1 ) */ } From 594f88d24369bce18c16f5fddfbfdff832ff5287 Mon Sep 17 00:00:00 2001 From: Emil Popov Date: Fri, 30 Jun 2023 12:06:04 -0400 Subject: [PATCH 6/9] makes the ipIP_TYPE_OFFSET define signed so asserts can properly check for negative values. --- source/include/FreeRTOS_IPv4_Private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index 5339c3ab8a..3a03edd52e 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -62,7 +62,7 @@ * tag support, ipIP_TYPE_OFFSET will shrink to 2. With the current design, the Ethernet header cannot be expanded to contain * more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0. */ -#define ipIP_TYPE_OFFSET ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - sizeof( UDPPacket_t ) ) +#define ipIP_TYPE_OFFSET ( (int32_t) sizeof( UDPHeader_t ) + (int32_t) sizeof( IPHeader_IPv6_t ) - (int32_t) sizeof( UDPPacket_t ) ) #include "pack_struct_start.h" struct xIP_HEADER From 522449232c7eb9b7038159ddfa205ac32415545f Mon Sep 17 00:00:00 2001 From: Emil Popov Date: Fri, 30 Jun 2023 12:26:12 -0400 Subject: [PATCH 7/9] Improves the comments a bit --- source/FreeRTOS_IP.c | 2 +- source/include/FreeRTOS_IPv4_Private.h | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/source/FreeRTOS_IP.c b/source/FreeRTOS_IP.c index 2dffc1922b..dd3d699101 100644 --- a/source/FreeRTOS_IP.c +++ b/source/FreeRTOS_IP.c @@ -1849,7 +1849,7 @@ static eFrameProcessingResult_t prvProcessIPPacket( const IPPacket_t * pxIPPacke eReturn = prvAllowIPPacketIPv4( pxIPPacket, pxNetworkBuffer, uxHeaderLength ); { - /* The IP-header type is copied to a location 6 bytes before the + /* The IP-header type is copied to a special reserved location a few bytes before the * messages starts. It might be needed later on when a UDP-payload * buffer is being used. */ pxNetworkBuffer->pucEthernetBuffer[ 0 - ( BaseType_t ) ipIP_TYPE_OFFSET ] = pxIPHeader->ucVersionHeaderLength; diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index 3a03edd52e..a9609aa9c7 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -54,13 +54,12 @@ * If however we use that same constant for an IPv4 packet, we end up somewhere in front of the Ethernet header. * In order to accommodate that, the Ethernet frame buffer gets allocated a bit larger than needed. * For IPv4 frames, prvProcessIPPacket() stores the version header field at a negative offset, a few bytes before the start - * of the Ethernet header. That IPv4 version field MUST be stored the same distance from the payload as in IPv6. Make sure that: - * ipUDP_PAYLOAD_IP_TYPE_OFFSET == ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which expands to: - * ( sizeof( UDPPacket_t ) + ipIP_TYPE_OFFSET ) != ( sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) ) which means that: + * of the Ethernet header. That IPv4 version field MUST be stored the same distance from the payload as in IPv6. * ipIP_TYPE_OFFSET must be equal to: sizeof( UDPHeader_t ) + sizeof( IPHeader_IPv6_t ) - ( sizeof( UDPPacket_t ) * In most situations, ipIP_TYPE_OFFSET will end up being equal to 6. If the Ethernet header is enlarged to include VLAN * tag support, ipIP_TYPE_OFFSET will shrink to 2. With the current design, the Ethernet header cannot be expanded to contain - * more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0. + * more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0 + * or storing of the IPv4 version byte will overwrite the Ethernet header of the frame. */ #define ipIP_TYPE_OFFSET ( (int32_t) sizeof( UDPHeader_t ) + (int32_t) sizeof( IPHeader_IPv6_t ) - (int32_t) sizeof( UDPPacket_t ) ) From f64c5803318f869342222445ef072952f8fa8f08 Mon Sep 17 00:00:00 2001 From: Emil Popov Date: Fri, 30 Jun 2023 12:47:57 -0400 Subject: [PATCH 8/9] Puts back the original code with emphasis that it is not required and only used for debugging. --- source/FreeRTOS_IP.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source/FreeRTOS_IP.c b/source/FreeRTOS_IP.c index dd3d699101..11dc3cbff9 100644 --- a/source/FreeRTOS_IP.c +++ b/source/FreeRTOS_IP.c @@ -1823,6 +1823,11 @@ static eFrameProcessingResult_t prvProcessIPPacket( const IPPacket_t * pxIPPacke /* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */ /* coverity[misra_c_2012_rule_11_3_violation] */ eReturn = prvAllowIPPacketIPv6( ( ( const IPHeader_IPv6_t * ) &( pxIPPacket->xIPHeader ) ), pxNetworkBuffer, uxHeaderLength ); + + /* The IP-header type is copied to a special reserved location a few bytes before the message + * starts. In the case of IPv6, this value is never actually used and the line below can safely be removed + * with no ill effects. We only store it to help with debugging. */ + pxNetworkBuffer->pucEthernetBuffer[ 0 - ( BaseType_t ) ipIP_TYPE_OFFSET ] = pxIPHeader_IPv6->ucVersionTrafficClass; } break; #endif /* ( ipconfigUSE_IPv6 != 0 ) */ From 9a63342a969574eae8056e29ab975528428fddfd Mon Sep 17 00:00:00 2001 From: GitHub Action Date: Mon, 3 Jul 2023 13:01:32 +0000 Subject: [PATCH 9/9] Uncrustify: triggered by comment. --- source/include/FreeRTOS_IPv4_Private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/include/FreeRTOS_IPv4_Private.h b/source/include/FreeRTOS_IPv4_Private.h index a9609aa9c7..a22d762e80 100644 --- a/source/include/FreeRTOS_IPv4_Private.h +++ b/source/include/FreeRTOS_IPv4_Private.h @@ -61,7 +61,7 @@ * more than one VLAN tag or ipIP_TYPE_OFFSET will become less than zero. ipIP_TYPE_OFFSET should never be allowed to be <= 0 * or storing of the IPv4 version byte will overwrite the Ethernet header of the frame. */ -#define ipIP_TYPE_OFFSET ( (int32_t) sizeof( UDPHeader_t ) + (int32_t) sizeof( IPHeader_IPv6_t ) - (int32_t) sizeof( UDPPacket_t ) ) +#define ipIP_TYPE_OFFSET ( ( int32_t ) sizeof( UDPHeader_t ) + ( int32_t ) sizeof( IPHeader_IPv6_t ) - ( int32_t ) sizeof( UDPPacket_t ) ) #include "pack_struct_start.h" struct xIP_HEADER