fix(microlink): worker task liveness bitmask, orphan-parked teardown - #47
Merged
Conversation
Replaces the fixed 3s sleep in microlink_stop() with a real join: each of the 4 core tasks (net_io, derp_tx, coord, wg_mgr) sets its ML_TASK_BIT_* before creation and clears it as the last thing it does before exiting, via the new ml_task_exit(). A zero mask is proof no worker can touch the context again, which is what microlink_destroy() now gates the free on — a task wedged in DNS or a TLS handshake on a captive-portal network routinely outlived the old fixed sleep, and woke up to a freed context (queues, event group) already reused, producing UAF crashes on real hardware. A context whose workers haven't joined by microlink_stop()'s timeout is parked in an orphan list instead of freed, reclaimed later by microlink_reap_orphans() (now called automatically from microlink_init()/microlink_destroy()) once its tasks finish draining. microlink_start() also gets a fail_start rollback path: if a task creation fails partway through, the tasks that did come up are joined via microlink_stop() instead of being abandoned as future orphans. Also fixes a real double-init leak in ml_derp_connect(): the mbedTLS ssl/ssl_conf structs were re-initialized on every retry without freeing the previous attempt's state first, orphaning heap allocations mbedTLS's dynamic-buffer plumbing still held pointers to. Adapted (not cherry-picked — base has diverged too far) from cplewes/microlink@a415d646 (bitmask/orphan mechanism) and cplewes/microlink@7120dfa4 (double-init fix; the rest of that commit's NULL-guard approach is superseded by a415d64's bitmask, per its own follow-up comments). Our fork's ml_derp_conn_t has no entropy/ctr_drbg fields (RNG is PSA-owned post mbedTLS 4.x migration), so the double-init fix frees only ssl/ssl_conf — same adaptation already used for derp_free_tls_state() in #44. Closes #21 Closes #22 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both landed in one PR since cplewes/microlink@a415d646 (issue #21) supersedes most of @7120dfa4 (issue #22) — see the PR description for the full breakdown of what was kept from each commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fudio101
added a commit
that referenced
this pull request
Aug 18, 2026
…e queue depth (#51) CONFIG_ML_CONFIG_HTTPD (default y) gates only the port-80 httpd task start; ml_config_httpd_init() still loads NVS settings regardless, so a provisioned device is unaffected with it off, saving ~7-8KB of internal RAM on RAM-tight boards. ML_PEER_UPDATE_QUEUE_DEPTH was an underived 400, allowing a theoretical ~80KB internal-RAM burst of in-flight peer-update payloads. 32 (4x runtime max_peers=8) caps that burst at ~6.6KB. Adapted from antmanler/microlink@6ef9f5a0 — same logic/comments, hand- ported rather than cherry-picked since the diff no longer applies cleanly after PR #47's teardown rewrite shifted surrounding lines. Closes #27 Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
4 tasks
fudio101
added a commit
that referenced
this pull request
Aug 19, 2026
…hot (#54) Adapted from Csontikka/microlink@5bda1783 (issue #32), with the already-fixed active-clearing bug (PR #53, this same session) kept removed -- the source commit's diff still cleared peer->active right after wireguardif_connect(), which PR #53 (issues #26/#28) proved permanently blocks that peer's direct-path handshake since active is the master session-initiation gate, not a retry throttle. Porting this verbatim would have silently reintroduced a bug fixed two PRs ago. process_disco_pong() fired a one-shot direct-path handshake init gated by a single boolean latch (tried_initial_handshake): if that one init was lost or unanswered, the peer stayed permanently un-sessioned on the direct path even though further direct PONGs kept arriving. Replaced the latch with a retry timestamp (last_init_handshake_ms) so a dropped/unanswered init gets another try every 30s (INITIAL_HANDSHAKE_RETRY_MS) instead of giving up forever. Also documented in FORK_PRS.md that issue #36 (wake blocked sockets on stop) was investigated and isn't needed here: this fork's microlink_stop() (from issues #21/#22's PR #47) never frees context until ml_join_tasks() proves every worker exited, and ml_derp.c's blocking loops already call ml_shutdown_pending() on every SO_RCVTIMEO-bounded iteration to bail out cooperatively -- the source commit's UAF doesn't exist in this fork's architecture. Closes #32 Co-authored-by: Adrian.Nguyen-Qualgo <nguyen.ndt@qualgo.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.
What
Replaces
microlink_stop()'s fixed 3svTaskDelaywith a real join, using aper-task liveness bitmask (
ML_TASK_BIT_NET_IO/DERP_TX/COORD/WG_MGR):xTaskCreatePinnedToCore()and clears it as the very last thing it does, via the new
ml_task_exit(),before
vTaskDelete(NULL).microlink_destroy()now gates the free on. A worker wedged in DNS or aTLS handshake on a captive-portal network routinely outlived the old fixed
sleep, then woke up to a freed context (queues, event group already
reused) — a use-after-free crash observed on real hardware.
ML_STOP_JOIN_TIMEOUT_MS(10s), thecontext is parked in an orphan list instead of freed, and reclaimed later
by the new
microlink_reap_orphans()(called automatically frommicrolink_init()/microlink_destroy()) once the straggler finallyexits.
microlink_start()gets afail_startrollback path: if a task creationfails partway through, the tasks that did come up are joined via
microlink_stop()instead of being abandoned as future orphans. Also logsfree internal/PSRAM heap at the failure point for diagnostics.
ml_shutdown_pending()checks were added at the 3 places a worker canblock for seconds during DERP connect/reconnect (the two retry loops in
ml_derp_tx_task, and the TLS handshakeWANT_READ/WANT_WRITEspin inml_derp_connect()), so teardown doesn't have to wait out a full DNS orTLS timeout.
Also fixes a real bug in
ml_derp_connect(): the mbedTLSssl/ssl_confstructs were re-initialized on every connect/retry without freeing the
previous attempt's state first — orphaning heap allocations that mbedTLS's
dynamic-buffer plumbing still held pointers to.
Source
Adapted (not cherry-picked — this fork's base has diverged too far from
upstream for a literal
git cherry-pick) from two commits oncplewes/microlink:a415d646— the liveness bitmask / orphan-park mechanism. Ported directly;our
microlink.c/microlink_internal.hstructure at this point matchedcplewes's closely enough for a near-verbatim port.
7120dfa4— by diff inspection this came beforea415d646in cplewes'sown history and added an ad-hoc
ml->events-NULL-guard +fail_startapproach;
a415d646is built directly on top of it and replaces most ofthose guards with the proper bitmask (its own comment in
ml_coord.csaysthe leftover NULL check is "belt-and-braces... used to be the only
guard"). The one piece of
7120dfa4not superseded — the DERP mbedTLSdouble-init fix — is included here as its own, distinct change.
Our fork's
ml_derp_conn_thas noentropy/ctr_drbgfields (RNG isPSA-owned post mbedTLS 4.x/TF-PSA-Crypto migration, see
ESP_IDF_6X_COMPAT.md), so the double-init fix frees onlyssl/ssl_conf— the same adaptation already used for
derp_free_tls_state()in #44.Scope note
ml_cellular.c,ml_net_switch.c, andml_udp.ceach have their ownvTaskDelete(NULL)call sites, deliberately not touched here — those arethe conditional net_switch/cellular-PPP task and a per-socket helper task,
outside the 4-core-task teardown contract this PR establishes (see
CLAUDE.md's task table).Related issues
Closes #21
Closes #22
Upstream reference: none — this is fork-mining work, not from
CamM2325/microlink's own PR queue (seeUPSTREAM_PRS.mdfor thattracking).
Test plan
idf.py buildisn't runnable in this sandbox (no ESP-IDF installed). Manualverification done instead:
grep -n "vTaskDelete(NULL)" components/microlink/src/ml_net_io.c components/microlink/src/ml_derp.c components/microlink/src/ml_coord.c components/microlink/src/ml_wg_mgr.creturns nothing — all 6 core-task exit points converted to
ml_task_exit().ml_cellular.c/ml_net_switch.c/ml_udp.cstill have their ownvTaskDelete(NULL), untouched (scope check).mbedtls_ssl_free/config_freepair at the top ofml_derp_connect()never touchesderp.sockfd, and the existingfail_tls:/derp_free_tls_state()cleanup path (from fix(derp,coord): absorb bugfix/hardening subset of upstream PR #22 #44) still runs onevery failure path afterward — no double
ml_close_sock()on the same fd.microlink_stop()/destroy()under normal conditions (should behave identically — joinshould complete near-instantly), and ideally a captive-portal or
DNS-blackhole scenario to exercise the join-timeout/orphan-park path,
which wasn't reachable in this sandbox.