From cf055c237a93d3308e1670285dfd2b629f0dd8af Mon Sep 17 00:00:00 2001 From: Cedric Van den Bergh Date: Wed, 8 Jul 2026 12:47:03 +0100 Subject: [PATCH 1/2] ncm: fix carrier lost on link-state notify collision tud_network_link_state() delivered the NETWORK_CONNECTION notification edge-triggered and fire-once: if a previous notification was still in flight, notification_xmit() returned early and the notification for the new link state was never queued. Because link_is_up is committed before the send, the host could be left reporting a stale carrier state - e.g. a permanent NO-CARRIER after a link up. The notification state was also mutated from both the caller and the notify xfer-completion callback with no serialisation, so on RTOS ports where tud_network_link_state() runs in a task other than tud_task() the two could race. Defer the whole link-state update onto the usbd task, so it can no longer race the completion callback. A collision with an in-flight notification is resolved by re-arming notification_xmit_state and letting the existing completion callback drive it forward on the next xfer completion, rather than adding a separate pending/retry flag. A link toggle does not change the link speed, so strictly only the NETWORK_CONNECTION notification needs (re)sending, but reusing the existing speed-then-connection state machine keeps the fix on a single, already-serialised code path. Closes #3760 --- src/class/net/ncm_device.c | 47 +++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/src/class/net/ncm_device.c b/src/class/net/ncm_device.c index 84a524f49..72b592787 100644 --- a/src/class/net/ncm_device.c +++ b/src/class/net/ncm_device.c @@ -800,31 +800,56 @@ static void tud_network_recv_renew_r(uint8_t rhport) { } // tud_network_recv_renew /** - * Set the link state and send notification to host + * usbd-task trampoline for tud_network_link_state(), packing rhport and is_up + * into a single pointer-sized argument. + * + * Runs entirely in the usbd task context, so it cannot race the notify + * xfer-completion callback over the notification state machine. Re-arming + * notification_xmit_state and kicking notification_xmit() (rather than + * sending NETWORK_CONNECTION directly) means a state change that collides + * with an in-flight notification is picked up by the existing completion + * callback instead of being silently dropped - which would otherwise leave + * the host stuck at NO-CARRIER after a link-state change. */ -void tud_network_link_state(uint8_t rhport, bool is_up) { - TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up); +static void ncm_link_state_task(void *param) { + uintptr_t const arg = (uintptr_t) param; + uint8_t const rhport = (uint8_t) (arg >> 1); + bool const is_up = (arg & 1u) != 0; if (ncm_interface.link_is_up == is_up) { - // No change in link state - return; + return; // no change in link state } ncm_interface.link_is_up = is_up; - // Only send notification if we have an active data interface if (ncm_interface.itf_data_alt != 1) { - TU_LOG_DRV(" link state notification skipped (interface not active)\n"); - return; + TU_LOG_DRV(" link state notification deferred (interface not active)\n"); + return; // data interface not active yet; SET_INTERFACE(alt=1) will notify } - // Reset notification state to send speed change notification first, then link state notification + // A link toggle does not change the link speed, so strictly only the + // NETWORK_CONNECTION notification would need (re)sending. Re-running the + // speed-then-connection sequence keeps this on the same state machine the + // completion callback already drives, at the cost of a redundant speed + // notification on every toggle. ncm_interface.notification_xmit_state = NOTIFICATION_SPEED; - - // Trigger notification transmission notification_xmit(rhport, false); } +/** + * Set the link state and notify the host. + * + * Defers onto the usbd task so a caller running in a different task than + * tud_task() cannot race the notification state machine against the notify + * xfer-completion callback. + */ +void tud_network_link_state(uint8_t rhport, bool is_up) { + TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up); + + uintptr_t const arg = ((uintptr_t) rhport << 1) | (is_up ? 1u : 0u); + usbd_defer_func(ncm_link_state_task, (void *) arg, false); +} + //----------------------------------------------------------------------------- // // all the netd_*() stuff (interface TinyUSB -> driver) From d0f8c75edd3f6f05792976dbdbb0bc21f4d8ed39 Mon Sep 17 00:00:00 2001 From: Cedric Van den Bergh Date: Wed, 8 Jul 2026 15:04:35 +0100 Subject: [PATCH 2/2] test/fuzz: stub usbd_defer_func in net_ncm harness The self-contained net_ncm fuzz harness #includes ncm_device.c and stubs the usbd symbols it references rather than linking the device stack. tud_network_link_state() now calls usbd_defer_func(), so add a matching no-op stub to keep the harness linking. --- test/fuzz/device/net_ncm/fuzz.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/fuzz/device/net_ncm/fuzz.c b/test/fuzz/device/net_ncm/fuzz.c index a93c144f7..3052636d8 100644 --- a/test/fuzz/device/net_ncm/fuzz.c +++ b/test/fuzz/device/net_ncm/fuzz.c @@ -47,6 +47,9 @@ bool usbd_open_edpt_pair(uint8_t rhport, uint8_t const *p_desc, uint8_t ep_count (void) rhport; (void) p_desc; (void) ep_count; (void) xfer_type; (void) ep_out; (void) ep_in; return true; } +void usbd_defer_func(osal_task_func_t func, void *param, bool in_isr) { + (void) func; (void) param; (void) in_isr; +} bool tud_control_xfer(uint8_t rhport, tusb_control_request_t const *request, void *buffer, uint16_t len) { (void) rhport; (void) request; (void) buffer; (void) len; return true;