From 2b387264cb6595722246f63b0b619ee17aebd8f3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 02:38:53 +0200 Subject: [PATCH 1/2] fix(M14): generation-guard the DNS callback; drop NTP datagrams outside WAITING Co-Authored-By: Claude Opus 4.8 (1M context) --- network/ntp.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/network/ntp.c b/network/ntp.c index 735d2d5..840a4ae 100644 --- a/network/ntp.c +++ b/network/ntp.c @@ -12,6 +12,7 @@ #include #include +#include #include // --------------------------------------------------------------------------- @@ -39,6 +40,11 @@ static uint8_t ntp_nonce[8]; // stored transmit timestamp for origin che static struct udp_pcb *ntp_pcb = NULL; static ip_addr_t server_addr; +// Bumped once per ntp_sync(); passed through the DNS callback arg so a delayed +// reply from an earlier, timed-out request is dropped instead of clobbering the +// state of a later sync (M14). +static uint32_t ntp_generation = 0; + static bool synced = false; static uint32_t last_sync_unix = 0; static uint64_t last_sync_monotonic_us = 0; @@ -115,6 +121,13 @@ static void apply_time(uint32_t unix_time) { static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip_addr_t *addr, u16_t port) { + // Drop datagrams that arrive outside a waiting window (M14): a response for + // a request we are no longer waiting on must not touch the state machine. + if (ntp_state != NTP_STATE_WAITING) { + pbuf_free(p); + return; + } + if (p->tot_len < 48) { // use tot_len not len (L10 fix too) printf("[ntp] response too short\r\n"); pbuf_free(p); @@ -198,6 +211,11 @@ static void ntp_recv_cb(void *arg, struct udp_pcb *pcb, struct pbuf *p, const ip // --------------------------------------------------------------------------- static void dns_found_cb(const char *name, const ip_addr_t *ipaddr, void *arg) { + // Drop a stale callback from an earlier, timed-out sync (M14): a delayed DNS + // reply must not clobber server_addr / nonce / state of a later request. + if ((uint32_t)(uintptr_t)arg != ntp_generation) + return; + if (!ipaddr) { printf("[ntp] DNS failed\r\n"); ntp_state = NTP_STATE_FAILED; @@ -253,13 +271,17 @@ bool ntp_sync(void) { udp_recv(ntp_pcb, ntp_recv_cb, NULL); ntp_state = NTP_STATE_RESOLVING; + // New request generation — any DNS callback carrying an older token is stale. + uint32_t generation = ++ntp_generation; + void *gen_arg = (void *)(uintptr_t)generation; + cyw43_arch_lwip_begin(); - err_t err = dns_gethostbyname(NTP_SERVER, &server_addr, dns_found_cb, NULL); + err_t err = dns_gethostbyname(NTP_SERVER, &server_addr, dns_found_cb, gen_arg); cyw43_arch_lwip_end(); if (err == ERR_OK) { // Already cached - fire callback manually - dns_found_cb(NTP_SERVER, &server_addr, NULL); + dns_found_cb(NTP_SERVER, &server_addr, gen_arg); } else if (err != ERR_INPROGRESS) { printf("[ntp] DNS error: %d\r\n", err); udp_remove(ntp_pcb); From 30ba15759f5bb914a1155599dbe62b48d10f4e42 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 20:26:42 +0200 Subject: [PATCH 2/2] fix(cmd_status): function-scope keys so the coverage/firmware build compiles Rebasing onto current master pulled in a latent build break: cmd_status declares `keys` inside the `if (commands_is_admin())` block but scrubs it via `secure_wipe(keys, sizeof(keys))` at function scope, so `keys` is undeclared there. asan_commands links serial/commands.c (not commands_system.c) so it never compiled this TU, but `make -C test coverage` compiles the whole first-party surface and fails here (`keys undeclared`) -- as does the real firmware build. Restore `keys` to function scope, matching the scrub's intent of always clearing the key DB from BSS (incl. the non-admin path where it stays zero-initialised). Same fix as PR #15 (M1). Co-Authored-By: Claude Opus 4.8 (1M context) --- serial/commands_system.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/serial/commands_system.c b/serial/commands_system.c index 4b92f9e..bdd3cac 100644 --- a/serial/commands_system.c +++ b/serial/commands_system.c @@ -64,11 +64,13 @@ void cmd_status(int argc, char **argv) { printf("ntp: not synced\r\n"); } - // Keys (admin only: key inventory is target-selection data) + // Keys (admin only: key inventory is target-selection data). Declared at + // function scope so the scrub below always runs, even on the non-admin path + // where the array stays zero-initialised. + static key_record_t keys[BACKUP_MAX_KEYS]; if (commands_is_admin()) { - static key_record_t keys[BACKUP_MAX_KEYS]; - int count = storage_key_list(keys, BACKUP_MAX_KEYS); - int enabled = 0, corrupt = 0; + int count = storage_key_list(keys, BACKUP_MAX_KEYS); + int enabled = 0, corrupt = 0; for (int i = 0; i < count; i++) { if (!keys[i].is_checksum_valid) corrupt++;