Skip to content

Child socket cleanup on TCP window allocation failure - #1372

Open
kstribrnAmzn wants to merge 3 commits into
FreeRTOS:mainfrom
kstribrnAmzn:childSocketCleanup
Open

kstribrnAmzn wants to merge 3 commits into
FreeRTOS:mainfrom
kstribrnAmzn:childSocketCleanup

Conversation

@kstribrnAmzn

Copy link
Copy Markdown
Member

Description

prvTCPSocketCopy() stores a pointer to a new child socket in the listening socket's u.xTCP.pxPeerSocket field, but when prvTCPCreateWindow() fails the IPv4 and IPv6 listen paths free that child through a direct vSocketClose() call that bypasses the vTCPRemoveTCPChild() cleanup. With this change, the reference is cleared before the socket is closed.

We thank Ghaith Abdulreda for this hardening suggestion.

Test Steps

Unit tested

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@kstribrnAmzn
kstribrnAmzn marked this pull request as ready for review October 2, 2026 22:16

@htibosch htibosch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @kstribrnAmzn and Ghaith Abdulreda, thank you for this PR.

I always enable hang protection:

#define ipconfigTCP_HANG_PROTECTION     ipconfigENABLE

and so the other path was "less tested".

Hein

prvTCPSocketCopy() stores a pointer to a new child socket in the listening
socket's u.xTCP.pxPeerSocket field, but when prvTCPCreateWindow() fails the
IPv4 and IPv6 listen paths free that child through a direct vSocketClose()
call that bypasses the vTCPRemoveTCPChild() cleanup. With this change, the
reference is cleared before the socket is closed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants