TCP: fix pre-accept connection handling and add wolfIP_sock_abort() - #178
Merged
Merged
Conversation
Member
Author
|
@wolfSSL-Fenrir-bot review balanced |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #178
Scan targets checked: wolfip-src, wolfip-bugs
Coverage: 1 of 2 in-scope changed file(s) opened by the reviewer; not opened: src/port/wolfssl_io.c
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Balanced
gasbytes
requested changes
Sep 29, 2026
wolfIP_sock_recvfrom() returned a bare -1 for a socket still in SYN_SENT or SYN_RCVD, and wolfIP_sock_sendto() did the same for SYN_SENT; that is the value both use for operations that can never succeed. A caller cannot tell "wait" from "give up": treat every negative as retryable and a real failure becomes an endless poll; treat -1 as fatal and a connection that is merely young dies. A non-blocking TLS server on the native API hits the second at once, because the first read after accepting from SYN_RCVD lands before the final ACK. Return -WOLFIP_EAGAIN in both handshake states, for both directions. sendto() already does this for SYN_RCVD since the late-accept work; this makes receive and SYN_SENT consistent with it, and matches what a non-blocking recv() returns on a connecting socket on Linux. LISTEN and the closing states keep -1: telling a caller to retry on a socket that is never coming back would spin for ever. A listener keeps -1 in SYN_RCVD too: can_read() reports a listener with a pending connection as readable, so EAGAIN there would send a caller that waits for readability round a loop that never makes progress. The wolfSSL I/O callbacks already map EAGAIN to WANT_READ/WANT_WRITE and -1 to a fatal close, so a handshake started on a connecting socket now waits instead of failing. Their comments said -1 meant "not established"; they now say it means a listener or a torn-down stream. The tests that pinned -1 for SYN_SENT now expect EAGAIN, and assert -1 for LISTEN instead so that path stays covered. docs/API.md gains the return contract, including that wolfIP_sock_close() returns EAGAIN while the FIN exchange is outstanding. The stack frees that descriptor by itself once the exchange ends, and a descriptor is only a slot index, so the doc also says that a repeated close is safe only until the next socket is created or accepted: after that it would close the new socket.
A connection whose handshake completed but which the application did not accept within TCP_PREACCEPT_TIMEOUT_MS was reclaimed silently. The peer had finished its own handshake and considered itself connected, so it waited on a connection this stack had already thrown away - no FIN, no RST - until its own timeout fired, if it had one. The expiry path assumed "the peer's next segment gets the normal LISTEN RST". That only holds for a peer with something left to send. A client waiting for the server to speak first has nothing: a TLS client that has sent its ClientHello and waits for the server's flight never sends another segment, so it never draws that RST. Measured on hardware, the peer hung for the full 45 s a test allowed, against a server that had dropped it 40 s earlier. tcp_send_reset_now() sends an RST|ACK straight through tcp_send_empty_immediate(). It cannot go through tcp_send_empty(): that queues into the socket's TX FIFO, and the listener revert that follows reinitialises the FIFO, so the segment would never leave. The segment is built by tcp_build_empty(), split out of tcp_send_empty() so both share it. Called from the two paths that discard an established pre-accept connection: the pre-accept expiry in tcp_rto_cb(), and accept() when no socket is free for the hand-off. The SYN_RCVD control-RTO revert is left alone; there the peer retransmits its SYN and draws the LISTEN RST. The connection is still lost. What changes is that the peer is told, and fails in about 5 s instead of hanging.
A listener that completes a handshake before the application accepts is carrying a connection while remaining the only socket bound to the port. If an RST arrived in that window, the generic reset path called close_socket() on it, and the port never answered again: one peer that connected, sent, and reset instead of closing took the service down for good. On hardware it did not come back within four minutes, and every later connection was refused. The SYN_RCVD case above already reverts a listener to LISTEN for this reason. This is the same socket one state later, so it reverts too. Only ESTABLISHED and CLOSE_WAIT revert, the states accept() can still hand off. A listener the application closed before accepting sits in FIN_WAIT_1 or LAST_ACK with is_listener still set; it has given the port up, and reverting it would leave a listener with no callback answering SYNs on a port the application may already have bound again. Those states keep the close_socket() teardown. Found by stress testing: a plaintext echo server died after ten connections that wrote and then reset, while a TLS server on another port of the same board kept serving. Only the echo server had a connection sitting un-accepted when the reset landed.
The pre-accept timer bounds how long a listener that completed a handshake can hold the port without the application accepting. It disarmed itself on any state other than ESTABLISHED, reading anything else as "accepted away, reset, or closed". CLOSE_WAIT is none of those: a peer that connects and closes without waiting - anything that writes a request and shuts down its write side - takes an un-accepted listener straight from ESTABLISHED to CLOSE_WAIT. The timer then disarmed, and until the application called accept() the port answered every other SYN with an RST. accept() now hands a CLOSE_WAIT listener off, so a server that gets round to it recovers. One that does not - busy, stuck, or not polling that descriptor - held the port indefinitely, while the same connection one FIN earlier would have been reclaimed after TCP_PREACCEPT_TIMEOUT_MS. Treat CLOSE_WAIT as still pinned, so the timer fires and reverts the listener as it does for ESTABLISHED.
wolfIP_sock_close() on a connected socket starts the FIN exchange and keeps the slot until the peer finishes it. A peer that stops reading, or aborts mid-handshake and never answers the FIN, can hold that slot for as long as the control retransmissions last - and on a stack with a handful of static sockets, one or two such peers is the difference between serving and refusing every new connection. A server that has decided a peer is dead has no way to say "drop it now". wolfIP_sock_abort() is that: SO_LINGER with a zero timeout. It sends an RST when the peer holds a synchronized connection (SYN_RCVD, ESTABLISHED, CLOSE_WAIT, FIN_WAIT_1/2, the same set Linux resets on abort) and releases the slot at once. A listener and an already-closed slot go through wolfIP_sock_close(), which releases those without a peer to tell. It is also valid after wolfIP_sock_close() returned -WOLFIP_EAGAIN, which is how a caller bounds a graceful close, as long as no socket has been created or accepted since: the stack frees a closing slot by itself, and its number can be handed out again. The RST carries SND.NXT as transmitted, not the seq cursor. seq runs ahead by queued, unsent data and is never advanced past the FIN, and a peer silently drops an RST below its RCV.NXT: aborting a socket stuck in FIN_WAIT_2 would free it here and leave the peer connected. tcp_snd_nxt() takes the end of the last transmitted segment from the TX FIFO, adds the FIN once it has gone out, and uses ISS+1 in SYN_RCVD. It errs high rather than low, because a peer answers an in-window RST at the wrong sequence with a challenge ACK, and the freed port answers that with an RST at the peer's own ACK number. Aborting a listener that still holds an un-accepted connection gives up the port, so it reports WOLFIP_FILT_STOP_LISTENING as well as WOLFIP_FILT_CLOSED. On a TC4D7 serving TLS from a two-socket pool, with the FreeRTOS wrapper's close() falling back to this after a bounded wait: ten consecutive aborted client handshakes, ten recoveries, against a build that refused every connection after the first.
danielinux
removed their request for review
September 29, 2026 10:33
wolfIP_sock_sendto() still returned -WOLFIP_EAGAIN for a listener in SYN_RCVD, while wolfIP_sock_recvfrom() already returned -1 there. Sending on a listener can never succeed, so a caller told to retry would loop for ever. The gap was wider than the handshake states. A connection that completes before accept() lives on the listener slot in ESTABLISHED or CLOSE_WAIT, and there sendto() queued data and advanced the sequence cursor, and recvfrom() consumed the pending connection's early data. accept() then copies that cursor into the child but discards the TX FIFO, so the accepted stream would run ahead of anything it could retransmit, and bytes read through the listener were lost to the accepted socket. A listener descriptor is never a data socket: is_listener is only ever cleared on the child accept() creates. sendto() and recvfrom() now return -1 for a listener in every state, and wolfIP_sock_can_write() reports the fixed readiness of 1 for it, as it already did in LISTEN, so a caller waiting for writability reaches the error instead of blocking.
Frauschi
force-pushed
the
tcp-preaccept-fixes
branch
from
September 29, 2026 10:55
36277af to
c5604db
Compare
gasbytes
approved these changes
Sep 29, 2026
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
4 participants
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.
Summary
A listener that completes a handshake before the application calls
accept()holds the connection on the listening socket itself, and with a handful of static sockets that window is where a server either keeps serving or goes dark. Stress testing a TLS server on a TC4D7 turned up several ways a single misbehaving peer could take a port down, leave the peer hanging, or pin a socket slot indefinitely. This PR fixes those paths in the core and adds an abortive close so an application can drop a peer it has given up on.-WOLFIP_EAGAIN, not-1.wolfIP_sock_recvfrom()returned-1inSYN_SENTandSYN_RCVD, andwolfIP_sock_sendto()did the same inSYN_SENT, so a caller could not tell "wait" from "give up". A non-blocking server hits this on its first read after accepting fromSYN_RCVD.LISTENand the closing states keep-1.accept()with no free socket both reclaimed an established connection silently. A client waiting for the server to speak first never sends another segment, so it never drew theLISTENRST and hung until its own timeout. Both paths now send an RST.ESTABLISHEDorCLOSE_WAITcalledclose_socket()on the only socket bound to the port, and the service never answered again. It now reverts toLISTEN, asSYN_RCVDalready did. A listener the application has already closed (FIN_WAIT_1/LAST_ACK) is still torn down.CLOSE_WAITstays bounded. A peer that connects, writes and shuts down beforeaccept()moves the listener toCLOSE_WAIT, where the pre-accept timer used to disarm itself and leave the port answering every other SYN with an RST. The timer now coversCLOSE_WAITtoo.wolfIP_sock_abort(). This is an abortive close, likeSO_LINGERwith a zero timeout. It sends an RST inSYN_RCVD,ESTABLISHED,CLOSE_WAITandFIN_WAIT_1/2(the set Linux resets;CLOSINGandLAST_ACKare released without one, as RFC 9293 specifies) and releases the slot at once. Listeners and closed slots go throughwolfIP_sock_close(). The RST carries SND.NXT as actually transmitted, not the send cursor, because a peer silently drops an RST below its RCV.NXT. Aborting a listener that holds an un-accepted connection also reportsWOLFIP_FILT_STOP_LISTENING.API changes
int wolfIP_sock_abort(struct wolfIP *s, int sockfd);wolfIP_sock_recv()/wolfIP_sock_recvfrom()on a socket inSYN_SENTorSYN_RCVD, andwolfIP_sock_send()/wolfIP_sock_sendto()inSYN_SENT, now return-WOLFIP_EAGAINinstead of-1. A listener keeps-1in every state, sincewolfIP_sock_can_read()reports a listener with a pending connection as readable. Callers that retry on-WOLFIP_EAGAINnow wait for a connection that is still being set up instead of dropping it; code that compared against-1specifically in these states needs to handle-WOLFIP_EAGAIN.wolfIP_sock_close()returns-WOLFIP_EAGAIN, the stack frees the descriptor on its own when the FIN exchange ends, and the number can be handed out again. A repeatedclose()orabort()on it is only safe until the next socket is created or accepted.Testing
unit_tests_tcp_flow.c, plus updated expectations in the API, socket-arm and DNS/DHCP suites for theEAGAINchange. The abort tests pin the RST sequence number in each state it covers, including after a data retransmission timeout, with a FIN queued behind unacknowledged data, and with a FIN re-queued by the control RTO. Each commit builds and passesmake uniton its own (Ubuntu 24.04, libcheck), 1621/1621 at the tip. Each regression test fails against the stack without its fix.close()falling back towolfIP_sock_abort()after a bounded wait, it recovered from ten consecutive aborted client handshakes. Before, it refused every connection after the first.-Werrorclean, the network test suite 10/10 (the pre-accept RST case resolves in 4 s), and the stress harness passes with the board still serving at the end.Docs
src/port/wolfssl_io.c: the I/O callback comments now describe theEAGAINcontract; the callbacks' behaviour is unchanged.docs/API.md:wolfIP_sock_abort(), the TCP return-value contract for the send/receive calls, and the descriptor-reuse caveat after aclose()that returned-WOLFIP_EAGAIN.