wireguardif: route decrypted RX through netif->input, not ip_input() (fixes pbuf double-free crash) - #20
Open
snowpaper wants to merge 1 commit into
Open
Conversation
snowpaper
force-pushed
the
fix/rx-via-tcpip-thread
branch
from
July 9, 2026 04:56
1be17d2 to
1f0b7dc
Compare
The decrypted-RX path called ip_input() directly on the caller's task,
entering the lwIP core concurrently with tcpip_thread. Under sustained
TCP traffic through the tunnel this double-frees a sent-segment pbuf
('pbuf_free: p->ref > 0' assert) and reboots the device.
ml_wg_mgr already sets netif->input = tcpip_input, i.e. the integration
intends all input serialized through the tcpip mailbox; the direct
ip_input() call bypassed that. Hand the pbuf to netif->input instead so
the lwIP core is only entered from tcpip_thread.
Verified on ESP32-S3 (ESP-IDF v5.3): multi-hundred-KB TCP relays that
previously crashed mid-transfer now run to completion.
Fixes CamM2325#17
snowpaper
force-pushed
the
fix/rx-via-tcpip-thread
branch
from
July 9, 2026 04:57
1f0b7dc to
e74be46
Compare
antmanler
added a commit
to antmanler/microlink
that referenced
this pull request
Aug 13, 2026
Captured working state from an always-on ESP32-S3 voice edge that runs this stack 24/7 against a WireGuard tailnet. Base: v2.1.0 (5a60e24). The themes, each proven on the live device: - TAI64N handshake timestamps come from the wall clock (gettimeofday), not esp_timer uptime. The RTC starts at zero after a power cycle, so uptime-based TAI64N dated handshakes ~55 years early and the peer refused every one — the classic symptom being "control plane connects, data plane dead". SNTP must complete before the first handshake; the consumer enforces that ordering. - Carries upstream PR CamM2325#20 (pbuf double-free in the WireGuard RX path) applied ahead of merge. - Task stacks and large buffers moved to PSRAM (MALLOC_CAP_SPIRAM) wherever the task never touches flash-cache-disabling operations: internal RAM on this board bottoms out at a few KB with the full audio pipeline up, and every internal allocation matters. - Coordinator / DERP / WG-manager diagnostics that print lengths and outcomes, never key material. This is a maintenance fork for the duoduo ambient edge client; changes are offered back upstream where they generalize. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fudio101
referenced
this pull request
in fugo101/microlink
Aug 14, 2026
Absorb upstream #20: route decrypted RX through netif->input
fudio101
added a commit
to fugo101/wireguard-lwip
that referenced
this pull request
Aug 14, 2026
…P compat, and lwIP thread-safety fixes (#3) Implements the callbacks declared in the previous commit, plus several independent fixes found and fixed together with that work: - DERP relay + magicsock output: wireguardif_peer_output() and wireguardif_device_output() now fall back to a DERP relay callback when a peer has no direct UDP endpoint (or force_derp_output is set), and route through an external magicsock UDP callback when one is registered instead of binding wireguard-lwip's own UDP PCB. wireguardif_init() gains a socket-less mode (wireguardif_disable_socket_bind()) for magicsock-only operation, and packets arrive via wireguardif_inject_packet() instead of udp_recv(). wireguardif_periodic() lets the caller drive handshake/ keepalive/rekey processing from its own task instead of the internal sys_timeout timer, since the X25519/ChaCha20-Poly1305 handshake work is too heavy to run unconditionally on the lwIP TCPIP thread. - Dual-stack IP compat: peer_lookup_by_allowed_ip() and wireguardif_device_output() take ip_addr_t instead of ip4_addr_t, via a small IP_ADDR_NETCMP_COMPAT/IP_ADDR_TO_IP4 shim (new lwip_compat.h) that bridges the ip_addr_t layout lwIP 2.2.0 (ESP-IDF 6.x) uses. Also fixes get_source_addr_port()'s IPv6 branch, which was gated on IP_IS_V4() instead of IP_IS_V6() and would never build a correct v6 cookie MAC key. - Keypair-aware allowed-IP fallback: when multiple peers match on allowed_source_ips, prefer one with a valid session keypair over one that only matches on the allowlist but can't actually exchange encrypted data yet (adapted from GrieferPig's fork). - fix: route decrypted RX through netif->input() instead of calling ip_input() directly. ip_input() enters the lwIP core immediately on whatever task called wireguardif_network_rx/wireguardif_periodic; when that isn't the tcpip_thread, it races the thread's own WiFi TCP processing over unsynchronized pbuf/PCB state and can double-free a pbuf. netif->input is tcpip_input, which posts the packet to the tcpip_thread's queue instead of processing inline. Upstream-PR: CamM2325/microlink#20 - fix: wireguardif_shutdown() cancels the periodic sys_timeout before the caller frees the device, and wireguardif_tmr()/wireguardif_periodic() null-guard netif/device — closes a use-after-free if the timer fires after teardown. - ESP-IDF 6.x compat: netif->state is temporarily NULLed around netif_set_link_up/down() calls. ESP-IDF 6.x registers a global lwIP netif ext callback that reads netif->state as esp_netif_t*; on the WireGuard netif that state is actually wireguard_device*, so the callback misinterprets it and crashes (LoadProhibited). NULLing state for the duration of the call makes the callback bail out cleanly. - Debug logging (WG_DEBUG_LOGGING, off by default) added throughout the RX/TX and handshake paths — this was invaluable for diagnosing the DERP/magicsock integration and is kept as a compile-time-gated tool for future debugging, not a permanent runtime cost. Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fudio101
pushed a commit
to fugo101/microlink
that referenced
this pull request
Aug 18, 2026
Extends the DERP TLS-leak fix already on this branch with the rest of what upstream shipped as one verified commit (125b529): - DERP region fallback when HomeDERP isn't in the DERPMap (falls back to the first usable non-avoid region instead of a dead-end connect to the default host) - Pin DERP TLS to 1.2 — some relays' Let's Encrypt ECDSA cert trips the ESP-IDF mbedTLS TLS 1.3 signature-algorithm OID path even under VERIFY_NONE - Check every mbedTLS setup call's return value (config_defaults, ssl_setup, set_hostname) and abort via the existing fail_tls path instead of proceeding into handshake with a half-initialized config - Rework the DERP BIO to a plain blocking f_recv bounded by SO_RCVTIMEO, dropping mbedtls_ssl_conf_read_timeout entirely — avoids a BAD_INPUT_DATA failure the recv_timeout scheme hit on some relays' TLS 1.3 path Plus the other generic (non-headscale) fixes from the same upstream PR: - Hostinfo "OS" field corrected to "esp32" (was "linux") - New ML_STATE_AUTH_FAILED state: do_register() now actually fails registration on a control-plane Error/MachineAuthorized rejection instead of silently "succeeding" into a broken state (Node absent, confusing downstream "node not found"); the expired-key-with-no- auth_key path raises the same state - state_cb now fires on the RECONNECTING transition (it never did before) and doesn't get clobbered by it while AUTH_FAILED is active Adapted from cplewes/microlink@38602ab0/@b25b1eee (already on this branch) and CamM2325#22 commits 125b529, fcdc8d9, ae3d438, 8367c1e, 9ef10bb — not a literal cherry-pick, base has diverged too far, and our mbedTLS 4.x/PSA migration already dropped the entropy/ctr_drbg fields upstream's version still frees. Deliberately excluded (see UPSTREAM_PRS.md): the 3 headscale/custom- control-plane feature commits from CamM2325#22 (this fork targets Tailscale's own SaaS control plane), and the netif->input RX fix (5c8d60c), already absorbed via CamM2325#20. Refs #14, #43 (FORK_PRS.md dj-oyu bundle's TAI64N item superseded), CamM2325#22 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fudio101
added a commit
to fugo101/microlink
that referenced
this pull request
Aug 18, 2026
* fix(derp): free the mbedTLS context on failed DERP connect ml_derp_connect()'s failure paths after the TLS phase begins only closed the raw socket — they never freed the mbedtls_ssl_context / mbedtls_ssl_config initialized just above. Each failed handshake leaked ~3-8 KB of internal heap, and microlink_rebind() reconnects DERP on every WiFi reconnect, so this compounds fast under captive-portal/bad-network conditions until no handshake can even allocate buffers. Extract derp_free_tls_state() (ssl/ssl_conf free + socket close) and route every post-init failure through a single `fail_tls` label via goto, reusing the same helper from ml_derp_disconnect()'s existing graceful-teardown path so the two call sites can't drift apart. Adapted from cplewes/microlink@38602ab0 and cplewes/microlink@b25b1eee. Not a literal cherry-pick: our ml_derp_conn_t already dropped the entropy/ctr_drbg fields as part of the mbedTLS 4.x / TF-PSA-Crypto migration (RNG is PSA-owned now), so derp_free_tls_state() frees only ssl/ssl_conf. Closes #14 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(derp,coord): absorb bugfix/hardening subset of upstream PR #22 Extends the DERP TLS-leak fix already on this branch with the rest of what upstream shipped as one verified commit (125b529): - DERP region fallback when HomeDERP isn't in the DERPMap (falls back to the first usable non-avoid region instead of a dead-end connect to the default host) - Pin DERP TLS to 1.2 — some relays' Let's Encrypt ECDSA cert trips the ESP-IDF mbedTLS TLS 1.3 signature-algorithm OID path even under VERIFY_NONE - Check every mbedTLS setup call's return value (config_defaults, ssl_setup, set_hostname) and abort via the existing fail_tls path instead of proceeding into handshake with a half-initialized config - Rework the DERP BIO to a plain blocking f_recv bounded by SO_RCVTIMEO, dropping mbedtls_ssl_conf_read_timeout entirely — avoids a BAD_INPUT_DATA failure the recv_timeout scheme hit on some relays' TLS 1.3 path Plus the other generic (non-headscale) fixes from the same upstream PR: - Hostinfo "OS" field corrected to "esp32" (was "linux") - New ML_STATE_AUTH_FAILED state: do_register() now actually fails registration on a control-plane Error/MachineAuthorized rejection instead of silently "succeeding" into a broken state (Node absent, confusing downstream "node not found"); the expired-key-with-no- auth_key path raises the same state - state_cb now fires on the RECONNECTING transition (it never did before) and doesn't get clobbered by it while AUTH_FAILED is active Adapted from cplewes/microlink@38602ab0/@b25b1eee (already on this branch) and CamM2325#22 commits 125b529, fcdc8d9, ae3d438, 8367c1e, 9ef10bb — not a literal cherry-pick, base has diverged too far, and our mbedTLS 4.x/PSA migration already dropped the entropy/ctr_drbg fields upstream's version still frees. Deliberately excluded (see UPSTREAM_PRS.md): the 3 headscale/custom- control-plane feature commits from CamM2325#22 (this fork targets Tailscale's own SaaS control plane), and the netif->input RX fix (5c8d60c), already absorbed via CamM2325#20. Refs #14, #43 (FORK_PRS.md dj-oyu bundle's TAI64N item superseded), CamM2325#22 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
antonmeyer
pushed a commit
to antonmeyer/microlink
that referenced
this pull request
Aug 20, 2026
wg_init_interface() already sets netif->input = tcpip_input specifically so RX code would dispatch through it instead of entering the IP stack directly - calling the netif's own configured input function (rather than hardcoding tcpip_input() again here) actually honors that existing setup instead of just swapping one hardcoded function for another that happens to currently match it. Same thread-safety fix as before, functionally identical in this integration, but follows the same pattern PR CamM2325#20 independently arrived at for this exact line, and matches how every other lwIP netif driver hands a received packet up the stack. Verified on real hardware again after the change: a full OTA push over the Tailscale tunnel (the same sustained-real-TCP-load scenario that originally crashed the board) completes cleanly, zero assertions.
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
1 participant
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.
Fixes #17.
The decrypted-RX path called
ip_input()directly on the caller's task, entering the lwIP core concurrently withtcpip_thread. Under sustained TCP traffic through the tunnel this double-frees a sent-segment pbuf (pbuf_free: p->ref > 0assert) and reboots the device mid-transfer.ml_wg_mgralready setsnetif->input = tcpip_input— the integration intends all input serialized through the tcpip mailbox; the directip_input()call bypassed that. This change hands the decrypted pbuf tonetif->inputso the lwIP core is only ever entered fromtcpip_thread.Verified on hardware (ESP32-S3 N16R8, ESP-IDF v5.3): a TCP proxy on the tailnet IP relaying ~600 KB streams crashed the board mid-transfer every time before this change; after it, jobs run to completion, and the node survived overnight idle plus a WAN modem restart. CRLF line endings preserved.