Fenrir fixes 2026 06 09#130
Merged
Merged
Conversation
wolfIP_poll's UDP TX drain loop popped descriptors from the udp.txbuf but never raised CB_EVENT_WRITABLE. wolfIP_notify_loopback_space_available() only covers loopback-interface sockets, so on a non-loopback UDP socket a sender that filled the txbuf (wolfIP_sock_sendto returns -WOLFIP_EAGAIN) and blocked waiting for CB_EVENT_WRITABLE was never woken once poll drained the queue (e.g. the FreeRTOS BSD shim sendto() blocking on xSemaphoreTake) -- a permanent deadlock. Set CB_EVENT_WRITABLE when the drain frees txbuf space, mirroring the loopback notify and the TCP drain path. Add a unit test that fills a non-loopback UDP txbuf to EAGAIN and asserts the bit is raised after poll. The ICMP drain loop has the same structural omission but is out of scope for this finding.
When a peer's final ACK acknowledges our FIN in TCP_LAST_ACK, tcp_ack() transitions to TCP_CLOSED and calls close_socket(), which memsets the tsocket (clearing ts->callback and ts->events) during packet processing, before wolfIP_poll() Step 3 dispatches socket callbacks. The pending CB_EVENT_CLOSED was therefore never delivered, so a caller blocked on the socket close (notably the FreeRTOS BSD close() waiting on xSemaphoreTake(portMAX_DELAY)) was never woken and deadlocked. Deliver CB_EVENT_CLOSED to a registered callback immediately before close_socket() destroys the socket, mirroring the poll Step 3 dispatch. Add a unit test that registers a callback on a LAST_ACK socket, injects the final ACK, and asserts CB_EVENT_CLOSED is delivered before teardown.
… to LISTEN A peer RST on a half-open connection in TCP_SYN_RCVD unconditionally reverted the socket to TCP_LISTEN. That is correct only for a real listening socket (is_listener=1) caught between SYN and accept(); an accepted clone produced by wolfIP_sock_accept() has is_listener=0 and is a distinct connection. Reverting it to LISTEN turned it into a phantom listener, cleared CB_EVENT_READABLE, and fired no callback, so a FreeRTOS BSD consumer blocked in recv() (state != ESTABLISHED -> -1 -> wait) never woke: the task deadlocked and the fd slot leaked, exhausting WOLFIP_FREERTOS_BSD_MAX_FDS after repetition. Gate the LISTEN revert on is_listener, mirroring the existing ctrl-RTO recovery path; otherwise deliver CB_EVENT_CLOSED synchronously (as the LAST_ACK teardown does, since close_socket() wipes the callback before poll Step 3) and close the socket. A subsequent recv() then observes TCP_CLOSED, wolfIP_sock_can_read() returns 1, and recv() returns an error instead of blocking forever.
The wolfSSL <-> wolfIP IO glue allocates a session from a static io_descs[MAX_WOLFIP_CTX] pool in wolfSSL_SetIO_wolfIP() but never released it: no cleanup function existed and httpd's teardown paths only freed the WOLFSSL object. Every TLS connection therefore leaked one slot, and after MAX_WOLFIP_CTX (8) connections every subsequent handshake failed until process restart - an unauthenticated DoS. The sibling wolfSSH/wolfMQTT ports already free their descriptors on teardown (io_desc_free / *_CleanupIO_*); the wolfSSL port did not. In addition, wolfIP_io_recv/wolfIP_io_send mapped a -1 return (the "not established" / torn-down case from wolfIP_sock_recvfrom / wolfIP_sock_sendto) to WANT_READ / WANT_WRITE, the same as the genuine would-block code -WOLFIP_EAGAIN. A reset connection was thus reported as retryable, so wolfSSL spun on a dead socket and the owning session was never closed (so its slot never reclaimed). Only -WOLFIP_EAGAIN is "would block"; -1 must be a fatal close. Add wolfSSL_CleanupIO_wolfIP() to release the slot, call it on every httpd TLS teardown path before wolfSSL_free(), and map -1 to a fatal close in both IO callbacks. Update the IO behavior unit tests for the new mapping and add test_wolfssl_io_cleanup_frees_slot proving a torn down session's slot becomes reusable.
wolfssh_io_send mapped both -WOLFIP_EAGAIN (TX buffer full, retryable) and -1 to WS_CBIO_ERR_WANT_WRITE. wolfIP_sock_sendto returns -1 when the TCP socket is no longer in ESTABLISHED/CLOSE_WAIT - the torn-down case after a peer RST - which is fatal, not "would block". Reporting it as WANT_WRITE made wolfSSH retry the dead connection forever, so the SSH state machine never transitioned to closing and the io_desc slot was never released via wolfSSH_CleanupIO_wolfIP. A handful of unauthenticated RSTs would exhaust the static io_descs[MAX_WOLFSSH_CTX] pool and permanently deny SSH service. This is the send-path sibling of the wolfSSL fix in F-5781. Map only -WOLFIP_EAGAIN to WS_CBIO_ERR_WANT_WRITE and route 0/-1 to WS_CBIO_ERR_CONN_CLOSE so the session is closed and its slot reclaimed. Add a wolfSSH IO unit harness (mock wolfssh/ssh.h plus stubs in unit_shared.c that include wolfssh_io.c with renamed static helpers) and test_wolfssh_io_send_behaviors, which asserts the -1 case now yields a fatal close. The test fails before the fix and passes after.
wolfssh_io_recv mapped both -WOLFIP_EAGAIN (no data queued, retryable) and -1 to WS_CBIO_ERR_WANT_READ. wolfIP_sock_recvfrom returns -1 when the TCP socket is no longer in ESTABLISHED/FIN_WAIT/CLOSE_WAIT (wolfip.c:6430-6431) - the torn-down case after a peer RST drives the socket to TCP_CLOSED - which is fatal, not "would block". Reporting it as WANT_READ propagates as WS_WANT_READ, and the SSH_STATE_KEY_EXCHANGE handler in ssh_server.c stays in KEY_EXCHANGE on WANT_READ/WANT_WRITE, so the handshake state machine is wedged forever. Because server.ssh is a single global and new connections are only accepted in SSH_STATE_LISTENING, one unauthenticated RST during key exchange permanently denies SSH service until reboot. This is the recv-path sibling of the send-path fix in F-5780. Map only -WOLFIP_EAGAIN to WS_CBIO_ERR_WANT_READ and route 0/-1 to WS_CBIO_ERR_CONN_CLOSE so the dead session is closed and its io_desc slot reclaimed, mirroring wolfssh_io_send. Add test_wolfssh_io_recv_behaviors, which asserts the -1 case now yields a fatal close. The test fails before the fix and passes after.
The stm32h563 tls_server.c example allocated a session from the static io_descs[MAX_WOLFIP_CTX] pool via wolfSSL_SetIO_wolfIP() but never released it: tls_client_free() called wolfSSL_shutdown/wolfSSL_free without first calling wolfSSL_CleanupIO_wolfIP(), so io_descs[i].stack stayed non-NULL and the slot was permanently marked in use. After MAX_WOLFIP_CTX (8) connect-then-abort cycles the pool was exhausted and every subsequent accept got a NULL IOCtx, disabling the TLS service until reboot - an unauthenticated DoS needing only a TCP handshake. F-5781 added wolfSSL_CleanupIO_wolfIP() and wired it into httpd's teardown paths, and ssh_server.c already frees its descriptor on teardown, but the stm32h563 tls_server.c call site was missed. Call wolfSSL_CleanupIO_wolfIP() before wolfSSL_free() in tls_client_free, and check the wolfSSL_SetIO_wolfIP() return value in tls_listen_cb, rejecting the accept (free SSL, close fd, release the client slot) when the pool is exhausted instead of proceeding with a NULL IOCtx.
When the hardware RNG fails, wolfIP_getrandom() fell back to an xorshift LFSR seeded from the compile-time constant 0x1A2B3C4D and never re-seeded, so every STM32H753 unit in the degraded state emitted the same globally known sequence (0xC9F6F11C, 0xED7A2436, ...). That makes TCP ISNs, ephemeral ports, DHCP xids and DNS ids predictable on any affected device. Seed the fallback LFSR from runtime state (DWT cycle counter plus any residual RNG_DR bits) on first use and mix the cycle counter in on each call, matching the convention already used by the lpc54s018 and va416xx ports (which use 0x1A2B3C4D only as a non-zero guard). Still not a cryptographic RNG, but no longer globally identical across devices.
wolfIP_getrandom() on the VA416xx port seeded a bijective xorshift32 LFSR once from HAL_time_ms (milliseconds since power-on) and mixed in no further entropy. The seed was therefore confined to the trivially-enumerable boot window, and because xorshift32 is bijective an on-path observer of a single output could invert the state and predict every subsequent value, including TCP ISNs, ephemeral source ports, DHCP xids and DNS ids. Mix the SysTick current-value register (a 24-bit free-running down-counter reloaded every 1ms) into both the initial seed and every call, matching the convention already used by the lpc54s018 and stm32h753 ports. This widens the initial state beyond the boot window and feeds fresh timing jitter on each call so a single observed output no longer determines the sequence. Still not a cryptographic RNG, but no longer enumerable or invertible from one observation.
esp_transport_wrap computed the payload length and ESP insertion point from the compile-time constant IP_HEADER_LEN (20), ignoring ver_ihl. For packets carrying IP options (IHL>5) -- e.g. igmp_send_report builds a Router-Alert packet with ver_ihl=0x46, and the forwarding path relays received optioned packets -- this over-counted orig_payload_len by the option length and inserted the ESP header at offset 20, overwriting the option bytes and dragging them into the ciphertext. The resulting frame is malformed (ver_ihl still claims IHL=6 while ESP starts after the fixed header) and fails AEAD/HMAC verification at a standards-compliant peer. Derive the real header length from ver_ihl and insert the ESP header after the IP header and its options (rfc4303 sec 3.1.1), keeping option bytes in the clear. Behaviour for the common IHL=5 case is unchanged. Add unit_esp regression test test_wrap_preserves_ip_options.
Every connection-close path in httpd.c freed the wolfSSL session without a preceding wolfSSL_shutdown(), so the close_notify alert required by RFC 5246 7.2.1 was never emitted, leaving HTTPS responses open to truncation attacks (CWE-325). Add wolfSSL_shutdown() before wolfSSL_CleanupIO_wolfIP()/wolfSSL_free() at all five close sites, matching the teardown sequence already used in tls_server.c, tls_client.c, mqtt_client.c and https_server_freertos.c. Add test-http-close-notify, which #includes httpd.c with stubbed wolfSSL teardown calls and asserts close_notify is sent before IO cleanup/free on each close path.
Contributor
There was a problem hiding this comment.
Pull request overview
Batch of networking, TLS/SSH IO, ESP, and embedded RNG fixes across wolfIP core, platform ports, and tests, aimed at improving correctness of teardown paths, protocol edge cases, and blocking-wakeup behavior.
Changes:
- Fix TCP/UDP callback/event delivery edge cases (CLOSED delivery on teardown; UDP WRITABLE after TX drain; correct SYN_RCVD RST handling for accepted sockets).
- Fix ESP transport encapsulation to respect IPv4 IHL (preserve IP options) and harden wolfSSL/wolfSSH custom IO behavior (treat
-1as fatal close; reclaim IO descriptor slots on teardown). - Improve embedded
wolfIP_getrandom()seeding/mixing on VA416xx and STM32H753, and expand regression/unit tests (incl. new standalone httpd close_notify test).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfip.h | Exposes wolfSSL_CleanupIO_wolfIP() in the public wolfSSL/wolfIP integration surface. |
| src/wolfip.c | Adjusts TCP teardown/callback delivery and SYN_RCVD RST behavior; wakes UDP senders after TX queue drain. |
| src/wolfesp.c | Uses actual IPv4 IHL when inserting ESP header in transport mode (preserves options). |
| src/http/httpd.c | Adds TLS close_notify + IO cleanup on multiple httpd TLS teardown/error-close paths. |
| src/port/wolfssl_io.c | Treats -1 as fatal close (not WANT_READ/WRITE) and adds IO-slot cleanup helper. |
| src/port/wolfssh_io.c | Treats -1 from wolfIP socket IO as fatal close for wolfSSH callbacks. |
| src/port/va416xx/main.c | Mixes SysTick jitter into VA416xx wolfIP_getrandom() seeding/output. |
| src/port/stm32h753/main.c | Seeds RNG fallback from runtime entropy (DWT cycle counter / RNG registers) and mixes jitter. |
| src/port/stm32h563/tls_server.c | Ensures wolfSSL IO slots are cleaned up on teardown and handles SetIO failure robustly. |
| src/test/unit/unit.c | Registers new/updated unit tests for the new behaviors and regressions. |
| src/test/unit/unit_tests_tcp_state.c | Adds coverage for SYN_RCVD RST handling differences (listener vs accepted socket). |
| src/test/unit/unit_tests_tcp_ack.c | Adds regression test ensuring CLOSED event delivery on LAST_ACK teardown. |
| src/test/unit/unit_tests_proto.c | Adds tests for IO slot cleanup and -1 close handling for wolfSSL/wolfSSH IO glue. |
| src/test/unit/unit_tests_poll_dispatcher.c | Adds regression test for UDP TX-drain setting WRITABLE. |
| src/test/unit/unit_shared.c | Extends unit-test mocks to support wolfSSL GetIOReadCtx + wolfSSH IO glue testing. |
| src/test/unit/unit_esp.c | Adds regression test confirming ESP wrap preserves IPv4 options (IHL > 5). |
| src/test/unit/mocks/wolfssl/ssl.h | Adds wolfSSL_GetIOReadCtx() to the unit-test wolfSSL mock API. |
| src/test/unit/mocks/wolfssh/ssh.h | Introduces wolfSSH mock header for unit tests covering wolfssh_io.c behavior. |
| src/test/test_http_smuggle.c | Adds stub for wolfSSL_CleanupIO_wolfIP() to keep standalone test linking. |
| src/test/test_http_arg_oob.c | Adds stub for wolfSSL_CleanupIO_wolfIP() to keep standalone test linking. |
| src/test/test_http_close_notify.c | New standalone regression test verifying close_notify ordering on all httpd close paths. |
| Makefile | Adds build target for test-http-close-notify standalone regression executable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…X path F-5788 (deliver CB_EVENT_CLOSED on LAST_ACK teardown) and F-5785 (close accepted socket on RST in SYN_RCVD) invoke the user socket callback *synchronously from deep inside tcp_input()/tcp_ack()*, i.e. at the bottom of the RX packet-processing call chain. The FreeRTOS BSD layer's callback calls printf(), which pulls in newlib's _vfprintf_r/_malloc_r; those frames on top of the already-deep RX stack overflow the wolfIP poll task's 1024-word stack. The result is an ARMv8-M stack-limit hardfault (CFSR=0x00100000 STKOF) faulting in the printf/malloc prologue, seen intermittently in the stm32h563 m33mu FreeRTOS echo CI job. Before F-5788 the same CB_EVENT_CLOSED was delivered from wolfIP_poll() Step 3, which dispatches socket callbacks from a shallow stack; that is the intended invariant. Restore it: - tcp_ack() LAST_ACK and tcp_input() SYN_RCVD-RST no longer call the callback or close_socket() directly. They set state = TCP_CLOSED and, when a callback is registered, OR in CB_EVENT_CLOSED and leave the socket in place so Step 3 delivers the event on a shallow stack and then reaps it via close_socket(). With no callback, the socket is closed immediately as before. - wolfIP_poll() Step 3 now dispatches a TCP_CLOSED socket only when it carries a deferred CB_EVENT_CLOSED, and reaps it after delivery. Other TCP_CLOSED sockets are still skipped (preserved by test_poll_tcp_cb_not_dispatched_when_closed). - tcp_input() ignores further input for a socket in the deferred-close window (TCP_CLOSED with CB_EVENT_CLOSED pending) so it cannot be matched/torn down before Step 3 delivers the event. Reproduced and verified on m33mu (stm32h563 FreeRTOS echo): with a 1KB frame added to the BSD callback the pre-fix code hardfaults (STKOF) 2/3 runs while the fixed code is clean 3/3; the real CI echo job passes. The two unit tests that asserted synchronous delivery now drive a poll cycle and assert the deferred delivery + teardown. All 1372 unit checks pass.
gasbytes
approved these changes
Jun 9, 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
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.
f1cae4a F-5732: send TLS close_notify before tearing down httpd sessions
71091fc F-5793: honour actual IHL in esp_transport_wrap for IP options
ba3ec3e F-5693: mix SysTick jitter into VA416xx wolfIP_getrandom
ca0f88b F-5695: seed STM32H753 RNG fallback LFSR from runtime entropy
f180cf6 F-5735: reclaim wolfSSL io_descs slot on stm32h563 TLS teardown
0ce610b F-5736: treat wolfIP_sock_recv -1 as fatal close in wolfssh_io_recv
5328f51 F-5780: treat wolfIP_sock_send -1 as fatal close in wolfssh_io_send
d52f8e1 F-5781: free wolfSSL io_descs slot on TLS teardown and treat -1 as fatal
4accd9b F-5785: close accepted socket on RST in SYN_RCVD instead of reverting to LISTEN
9182261 F-5788: deliver CB_EVENT_CLOSED on LAST_ACK teardown
7fe6bac F-5792: wake non-loopback UDP senders after wolfIP_poll txbuf drain