Skip to content

TCPIP: make the return value of network_socket_accept_tcp more expressive - #107

Merged
davidchisnall merged 1 commit into
CHERIoT-Platform:mainfrom
nding0405:fix-accept-return-value
Aug 8, 2026
Merged

TCPIP: make the return value of network_socket_accept_tcp more expressive#107
davidchisnall merged 1 commit into
CHERIoT-Platform:mainfrom
nding0405:fix-accept-return-value

Conversation

@nding0405

Copy link
Copy Markdown
Contributor

The network_socket_accept_tcp function can only return either a valid sealed capability or an untagged nullptr, so the caller cannot distinguish a timeout from an out-of-memory failure. Change it so that it can return an invalid capability that encodes the error code.

@nding0405
nding0405 force-pushed the fix-accept-return-value branch from 860576b to bd3fa50 Compare July 23, 2026 19:25
nding0405 added a commit to nding0405/network-stack that referenced this pull request Jul 23, 2026
This patch completes the fix for issue CHERIoT-Platform#104. PR CHERIoT-Platform#107 added an
ephemeral claim in with_sealed_socket(), protecting the
SealedSocket wrapper until the first cross-compartment call. In
network_socket_close(), that protection ends at heap_can_free().
In the post-reset close path, the socket lock cannot be acquired, so
any later dereference of the wrapper may race with another thread
freeing the wrapper and cause a Use-After-Free. The normal close path
is not affected because the wrapper remains protected by socketLock.

To remove this race, the fields needed after heap_can_free() are
copied from the wrapper before the cross call, and the wrapper is not
dereferenced afterward in the post-reset path. The raw FreeRTOS socket
is also vulnerable in rawSocket->ucProtocol; because this value is
needed only for the normal close path, but the current codebase make it
accesible for both nomal close and post-reset close, ane the later path
can produce an UAF on that line of code. Thus, to fix this, the dereference
is moved inside the lock-protected branch.

These changes eliminate both wrapper and raw-socket UAF from the unprotected
post-reset path.
@nding0405 nding0405 closed this Jul 29, 2026
@nding0405 nding0405 reopened this Jul 29, 2026

@davidchisnall davidchisnall 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.

Looks good, minor comment on a comment.

Comment thread include/NetAPI.h Outdated
Comment on lines +240 to +244
* -ENOMEM: allocation of the wrapper timed out or does not have memory for now.
* -EINVAL: The timeout pointer is invalid, or FreeRTOS_accept returns invalid
* socket, or fail to claim the socket poiting to mallocCapability,
* or fail to add socket to the socket reset list,
* -ETIMEDOUT: timed out on FreeRTOS_accept.

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.

Please can you put backticks around the values (e.g. -ENOMEM) and make them a Markdown list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi David, since #107 has been approved. Would you mind merging it when you have a chance? Once it is merged I will rebase #109 and re-request review. Thanks!

davidchisnall pushed a commit that referenced this pull request Jul 30, 2026
This patch completes the fix for issue #104. PR #107 added an
ephemeral claim in with_sealed_socket(), protecting the
SealedSocket wrapper until the first cross-compartment call. In
network_socket_close(), that protection ends at heap_can_free().
In the post-reset close path, the socket lock cannot be acquired, so
any later dereference of the wrapper may race with another thread
freeing the wrapper and cause a Use-After-Free. The normal close path
is not affected because the wrapper remains protected by socketLock.

To remove this race, the fields needed after heap_can_free() are
copied from the wrapper before the cross call, and the wrapper is not
dereferenced afterward in the post-reset path. The raw FreeRTOS socket
is also vulnerable in rawSocket->ucProtocol; because this value is
needed only for the normal close path, but the current codebase make it
accesible for both nomal close and post-reset close, ane the later path
can produce an UAF on that line of code. Thus, to fix this, the dereference
is moved inside the lock-protected branch.

These changes eliminate both wrapper and raw-socket UAF from the unprotected
post-reset path.
@nding0405
nding0405 force-pushed the fix-accept-return-value branch 2 times, most recently from e8a89ef to 88a3bea Compare July 31, 2026 01:20
…sive

The network_socket_accept_tcp function can only return either a valid
sealed capability or an untagged nullptr, so the caller cannot distinguish
a timeout from an out-of-memory failure. Change it so that it can return
an invalid capability that encodes the error code.
@davidchisnall
davidchisnall force-pushed the fix-accept-return-value branch from 88a3bea to d6a3d9d Compare August 8, 2026 10:12
@davidchisnall
davidchisnall merged commit c9ccde0 into CHERIoT-Platform:main Aug 8, 2026
8 checks passed
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