From 5d0908b23fca2406c26c31aa9cc0fc45bff24fc4 Mon Sep 17 00:00:00 2001 From: Andy Bennett Date: Fri, 25 Sep 2020 05:53:43 +0100 Subject: [PATCH] FreeRTOS+TCP Fix usGenerateChecksum on 8-bit platforms with odd-aligned buffers (#286) * Change type of usGenerateChecksum's ulAlignBits to intptr_t Not all platforms have 32-bit pointers. 8-bit machines such as avr8 have 16-bit pointers. This patch changes the type of ulAlignBits and renames it to uxAlignBits to reflect the type change. This fixes a compiler warning on machines with pointers that aren't 32-bits. Signed-off-by: Andy Bennett * Fix usGenerateChecksum on odd-aligned buffers with non zero usSum usGenerateChecksum would generate an incorrect checksum when pucNextData was odd-aligned and usSum was non-zero. This was caused by the byte order of usSum not matching the byte order of the subsequent summing operation. Odd-aligned buffers are common on 8-bit platforms such as avr8 when using one of the FreeRTOS dynamic heap allocators. Signed-off-by: Andy Bennett * Feedback from PR#122 https://github.com/FreeRTOS/FreeRTOS/pull/122 + Use a uintptr_t rather than an intptr_t. Changes supplied by Hein Tibosch. Signed-off-by: Andy Bennett Co-authored-by: Aniruddha Kanhere <60444055+AniruddhaKanhere@users.noreply.github.com> --- .../Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c index 1073a32b6..aa0513d89 100644 --- a/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c +++ b/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/FreeRTOS_IP.c @@ -2430,7 +2430,8 @@ aid though to optimise the calculations. */ xUnion32 xSum2, xSum, xTerm; xUnionPtr xSource; xUnionPtr xLastSource; -uint32_t ulAlignBits, ulCarry = 0UL; +uintptr_t uxAlignBits; +uint32_t ulCarry = 0UL; uint16_t usTemp; size_t uxDataLengthBytes = uxByteCount; @@ -2444,12 +2445,19 @@ size_t uxDataLengthBytes = uxByteCount; xTerm.u32 = 0UL; xSource.u8ptr = ipPOINTER_CAST( uint8_t *, pucNextData ); - /* coverity[misra_c_2012_rule_11_4_violation] */ - /* The object pointer expression "pucNextData" of type "uint8_t const *" is cast to an integer type "unsigned int". */ - ulAlignBits = ( ( ( uint32_t ) pucNextData ) & 0x03U ); /*lint !e9078 !e923*/ /* gives 0, 1, 2, or 3 */ + uxAlignBits = ( ( ( uintptr_t ) pucNextData ) & 0x03U ); + /* + * If pucNextData is non-aligned then the checksum is starting at an + * odd position and we need to make sure the usSum value now in xSum is + * as if it had been "aligned" in the same way. + */ + if( ( uxAlignBits & 1UL) != 0U ) + { + xSum.u32 = ( ( xSum.u32 & 0xffU ) << 8 ) | ( ( xSum.u32 & 0xff00U ) >> 8 ); + } /* If byte (8-bit) aligned... */ - if( ( ( ulAlignBits & 1UL ) != 0UL ) && ( uxDataLengthBytes >= ( size_t ) 1 ) ) + if( ( ( uxAlignBits & 1UL ) != 0UL ) && ( uxDataLengthBytes >= ( size_t ) 1 ) ) { xTerm.u8[ 1 ] = *( xSource.u8ptr ); xSource.u8ptr++; @@ -2458,7 +2466,7 @@ size_t uxDataLengthBytes = uxByteCount; } /* If half-word (16-bit) aligned... */ - if( ( ( ulAlignBits == 1U ) || ( ulAlignBits == 2U ) ) && ( uxDataLengthBytes >= 2U ) ) + if( ( ( uxAlignBits == 1U ) || ( uxAlignBits == 2U ) ) && ( uxDataLengthBytes >= 2U ) ) { xSum.u32 += *(xSource.u16ptr); xSource.u16ptr++; @@ -2539,7 +2547,7 @@ size_t uxDataLengthBytes = uxByteCount; /* coverity[value_overwrite] */ xSum.u32 = ( uint32_t ) xSum.u16[ 0 ] + xSum.u16[ 1 ]; - if( ( ulAlignBits & 1U ) != 0U ) + if( ( uxAlignBits & 1U ) != 0U ) { /* Quite unlikely, but pucNextData might be non-aligned, which would mean that a checksum is calculated starting at an odd position. */