[ritzylab-dev]

The bugs (post-merge review of #183)

The TLS connect worker released the slot, then read its completion callback and user context from the reusable
per-slot argument table; a second attempt accepted in that interval replaced them, so the first completion was lost
and the second callback ran twice. NVM mutation and flush calls returned success when the flash write had failed, and a
provisioning retry found the value in RAM and returned success without writing it.

What breaks, and when

A completion delivered to the wrong object, or never; a device that believes it is provisioned and boots without its
identity after power loss.

What changed and why it fixes it

The connect worker copies its attempt — host, port, SNI, callback, context — before any blocking work and uses only
that copy; nothing after the slot is released touches shared storage. Persisting now returns whether the table
generation reached flash; every mutation API and flush return that result and a failed write emits one
NVM_PERSIST_FAIL event. Provisioning is idempotent on the value, not on the attempt: an already-provisioned key returns
success only once its value is on flash, retrying a persist that failed.
Deleting an absent key likewise returns success only once the table without it is on flash, and TLS session
teardown has one owner, so a peer-closed session is released before recv() returns and is never re-initialised over.
Each TLS slot keeps two configuration sets: configure() builds and validates the replacement apart from the active
set, which a live session references and which is never modified or freed until that session ends; the next
connect_async() promotes it.

Evidence

Nineteen new tests (fifteen failing before their fix, four covering staged-set replacement, discard, promotion after a failed handshake and mutual TLS) passing after; the reviewer's probes rebuilt: callbacks 0/2 → 1/1; failed provision
now returns an error and the retry performs exactly one flash write with the identity readable after reload.

Limits stated

A failed persist does not move the NVM Functional Block to FAULT; that policy (permanent failure, retry budget) is a
separate decision. The in-memory table keeps the change — getters return it — and the next successful persist carries
it. Factory reset that fails part-way leaves the remaining keys in place until reboot (pre-existing; recorded).
Loading the table from flash is boot-only by contract: a runtime reload would discard a concurrent writer's
unpersisted change, so it is not supported.
If flash cannot be read at boot the store starts empty and refuses every mutation (NVM_PERSIST_FAIL, state
FLASH_UNREADABLE) until embediq_nvm_flush() re-reads it, so unknown flash contents are never overwritten; an invalid
blob (bad magic or CRC) is replaced by the first successful write.
The POSIX TLS HAL holds two configuration sets per slot (memory: certificates and keys for both); a staged
replacement is discarded by a later rejected configure().

🤖 Generated with Claude Code

[ritzylab-dev]

Reviewed PR #186 at aeead7a9a1be7b52931e53429361cb7a05ce1c14. One merge-blocking durability gap remains.

[P1] Retry an unpersisted deletion even when the key is already absent from RAM. In embediq_nvm_delete() lines 502–505, the absent-key fast path returns OK without checking persistence. Persist a key, fail the flash write for its deletion, recover flash, then retry the same deletion: the first call returns ERR, the retry returns OK with zero flash writes, and reloading restores the deleted value. I reproduced this against the exact head libraries using an in-memory HAL flash implementation:

seed=0 failed_delete=-1 retry=0 retry_writes=0 get_after_reload=0

get_after_reload=0 means the old value is present again. This is the same cache-versus-durability distinction the provision retry now correctly handles; a successful deletion retry must establish durable absence. Capture the current generation while holding the table lock and pass it through nvm_persist(cur, false) after unlocking on the absent-key path (as provision does), or implement equivalent durability tracking. Add a regression proving: failed deletion returns ERR; a retry while flash remains failed still returns ERR; recovery plus retry persists the removal; reload leaves the key absent; an already durable absence remains idempotent without an unnecessary write. This also affects embediq_cfg_delete().

The TLS attempt-local callback/context fix and connected-slot BUSY guard look correct in the reviewed paths. The focused local TLS/NVM/provisioning tests passed (4/4); CI run 41548772849 is green at this exact head, with Linux 33/33 and TSan zero reports, distinct findings, and runtime aborts. Those checks do not cover deletion retries. No source changes or merge performed.

[ritzylab-dev]

Fixed in 06014ed (test in 1fb4b0f, failing on aeead7a). embediq_nvm_delete() on an absent key now reads the current generation under the table lock and returns nvm_persist(cur, false), as provision does, so it returns OK only once the table without the key is on flash: a no-op when that is already true, one real write when an earlier delete's persist failed. Early-return audit of the mutation APIs: every OK from set, provision, delete, flush and reset now comes from nvm_persist(); the only literal return EMBEDIQ_OK lines are inside nvm_persist itself (the skip when a newer table is already on flash, and a successful write) and in get, a pure read. The new regression covers failed delete → ERR, a retry while flash still fails → ERR, recovery plus retry → OK with exactly one write, the key absent after reload, and a never-stored key → OK with zero writes. Your probe rebuilt against the fix: seed=0 failed_delete=-1 retry=0 retry_writes=1 get_after_reload=-1 (it was retry_writes=0 get_after_reload=0).

[ritzylab-dev]

Consolidated follow-up review at 55335a0: the deletion retry, attempt-local completion, session teardown, and unreadable-flash refusal/recovery fixes are addressed in the reviewed paths. One blocker remains: active TLS configuration lifetime.

[P1] Keep the active session’s entire configuration alive and immutable during configure(). hal_tls_posix.c lines 270–282 still frees/rebuilds s->conf, CA/client certificates, key, RNG and entropy while s->ssl retains the pointer supplied to mbedtls_ssl_setup(). Preserving the socket/SSL allocations alone does not preserve the session. The bundled mbedTLS ssl.h explicitly says no configuration copy is made and it must not be modified or freed while active. The mutex prevents simultaneous access but does not satisfy that lifetime requirement. Ordinary echo traffic passing is insufficient to establish safe credential rotation.

A white-box lifetime probe, compiled with the exact build’s mbedTLS configuration, configures a valid CA, calls ssl_setup, arranges an active slot, then calls configure with invalid PEM. It shows the still-connected SSL context now points to the failed replacement configuration, with its original CA gone:

initial_configure=0 setup=0 session_still_connected=1 failed_configure=-6 active_conf_is_shared=1 ca_before=1 ca_after=0

This probe demonstrates mutation of the retained configuration, not a simulated network handshake or a claimed exploitable crash. It confirms the stated first limit is a correctness issue, not an acceptable release limitation under the existing “current session is unaffected” contract.

Required closure: stage and validate replacement credentials/configuration separately, retain the active configuration and everything it references until the active SSL context is freed, then apply the replacement to the next session. A rejected reconfiguration must leave the active configuration intact. Add regressions for successful and failed reconfiguration while connected: both preserve active configuration/dependencies and ongoing traffic; a successful replacement is used by the next connection; repeat rotations/failures/reconnects/disconnects release all configurations/sessions exactly once. Verify both lifetime invariants and behavior; echo alone misses this defect.

The second stated limit (erased flash semantics) is a nonblocking HAL contract clarification for future ports: a successful erased-region read should return erased bytes, not an I/O failure. POSIX already implements usable fresh-device behavior; retain the new fail-closed handling for real read errors.

CI run 41722868070 is green at this exact head: Linux 33/33, TSan 0 reports / 0 distinct findings / 0 runtime aborts. I reviewed the accumulated PR and the latest failure paths; no other merge-blocking finding identified. This is the consolidated remaining fix and acceptance criteria. No source edits or merge performed.

[ritzylab-dev]

Fixed in b9b1346 (tests in 71594d1, failing on 55335a0). Each slot now holds two configuration sets: the active set — the one mbedtls_ssl_setup() was given, carried into the connect attempt by index — is never modified or freed while a session references it; configure() builds and validates into the scratch set, which becomes active at once when no session is live and is otherwise staged and promoted by the next connect_async() after the session is freed. A rejected configure() frees only the scratch set, so the active configuration, its CA and any session are untouched (a previously staged replacement is discarded). All configuration frees are in one helper. Your probe, adapted to s->cfg[s->active].conf: before initial_configure=0 setup=0 session_still_connected=1 failed_configure=-6 active_conf_is_shared=1 ca_before=1 ca_after=0, after initial_configure=0 setup=0 session_still_connected=1 failed_configure=-6 active_conf_is_shared=1 ca_before=1 ca_after=1. New regressions cover a successful and a rejected reconfigure while connected (same referenced configuration, CA present, traffic continues), the staged replacement being the next connection's configuration (a non-signing CA makes the next handshake fail cert verification), and 20 rotations alternating valid and rejected credentials across reconnects ending with one live configuration, zero live sessions and zero leaks.

[ritzylab-dev]

Final review at b9b1346: the active TLS configuration lifetime blocker is resolved. Each slot has separate active/scratch configuration storage; configure builds only the scratch set, failure frees scratch without changing active, and connect promotes a validated replacement only after the old session is freed. The retained configuration, credentials, entropy and RNG remain at stable addresses for the session lifetime. The documented failed-configure behavior on a disconnected slot is consistent with retaining the last valid configuration.

My adapted independent lifetime probe now reports failed_configure=-6 active_conf_is_shared=1 ca_before=1 ca_after=1 (previously ca_after=0). New regressions cover active session preservation, failed replacements, promotion of a different CA, and repeated rotation/teardown. Local NVM, config and provisioning checks passed; TLS passed on rerun (44/44 assertions). The first local TLS run had one missed connection in the 20-rotation test (19/20 connects/echoes); the unchanged rerun passed. The shared test helper has a fixed 200ms server startup delay and process-wide server cleanup; the exact cause of that intermittent failure was not established, so this is recorded as a test reliability follow-up rather than claimed repeatably clean.

Exact-head CI 41727840441 passed Linux 33/33 and TSan 33/33, with 0 report files, 0 findings, 0 runtime aborts. No remaining merge-blocking code findings identified in the reviewed PR. All previously raised blockers are addressed. Proceeding with the user-authorized merge of this exact head into dev.