Make sure that a TCP socket is closed only once#705
Closed
htibosch wants to merge 1 commit intoFreeRTOS:mainfrom
Closed
Make sure that a TCP socket is closed only once#705htibosch wants to merge 1 commit intoFreeRTOS:mainfrom
htibosch wants to merge 1 commit intoFreeRTOS:mainfrom
Conversation
aggarg
approved these changes
Feb 8, 2023
shubnil
approved these changes
Feb 10, 2023
Member
|
Closing this PR since PR #707 took care of fixing unit tests on top of this PR and got merged. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
If during the SYN phase, a sockets goes to the
eCLOSEDstate, it must be closed/released. But at that moment, it may happen that the application does a successful call toFreeRTOS_accept(), and gets the ownership of the socket.The IP-task in a meanwhile is planning to close the socket as well. See the function
vSocketCloseNextTime()Here is the proposed change:
if( ( eTCPState == eCLOSED ) || ( eTCPState == eCLOSE_WAIT ) ) { /* Socket goes to status eCLOSED because of a RST. * When nobody owns the socket yet, delete it. */ + vTaskSuspendAll(); { if( ( pxSocket->u.xTCP.bits.bPassQueued != pdFALSE_UNSIGNED ) || ( pxSocket->u.xTCP.bits.bPassAccept != pdFALSE_UNSIGNED ) ) { + if( pxSocket->u.xTCP.bits.bReuseSocket == pdFALSE_UNSIGNED ) + { + /* Make sure the the ownership will not be transferred. */ + pxSocket->u.xTCP.bits.bPassQueued = pdFALSE_UNSIGNED; + pxSocket->u.xTCP.bits.bPassAccept = pdFALSE_UNSIGNED; + } + xTaskResumeAll(); FreeRTOS_debug_printf( ( "vTCPStateChange: Closing socket\n" ) ); if( pxSocket->u.xTCP.bits.bReuseSocket == pdFALSE_UNSIGNED ) { /* Plan to close the socket. */ vSocketCloseNextTime( pxSocket ); } } + else + { + xTaskResumeAll(); + } } }The application can "take" the socket until
vTaskSuspendAll()is called. After that the IP-task will "take the ownership" by clearing bothbPassbits.This PR was tested in a set-up where the DUT received a TCP reset packet during the SYN phase.
Thanks Paul Helter, who first discovered and described this problem in PR #571. Mentioned PR was not merged because I missed a proper diagnosis of the cause. Only last week we saw exactly what happened, and could repair it.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.