From 886c6fac7e389d4c192cf42545c7ac4c2dfe90dd 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: retry link-state notification, fix carrier lost on 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 dedup at the top then suppressed any resend, so the host kept the stale carrier state - e.g. a permanent NO-CARRIER after a link up. The notification state (notification_xmit_state / _is_running / link_is_up) 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 race. Make link-state delivery level-triggered and race-safe: - add link_notify_pending, set when the link state changes and cleared only when a NETWORK_CONNECTION carrying the current state is actually submitted; - re-fire from the notify xfer-completion callback, so a send that collides with an in-flight notification is retried on the next completion instead of being silently dropped; - defer the send onto the usbd task via usbd_defer_func(), so a caller on another task cannot race the completion callback. A link toggle does not change the link speed, so only NETWORK_CONNECTION is re-sent on a change (CONNECTION_SPEED_CHANGE is still sent at SET_INTERFACE as before). Closes #3760 --- src/class/net/ncm_device.c | 58 ++++++++++++++++++++++++++++++++------ 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/src/class/net/ncm_device.c b/src/class/net/ncm_device.c index 84a524f492..f1865965f9 100644 --- a/src/class/net/ncm_device.c +++ b/src/class/net/ncm_device.c @@ -99,6 +99,7 @@ typedef struct { } notification_xmit_state; // state of notification transmission bool notification_xmit_is_running; // notification is currently transmitted bool link_is_up; // current link state + bool link_notify_pending; // host not yet (re)told the current link state // host-configured transmit limits uint8_t bm_capabilities; @@ -237,6 +238,7 @@ static void notification_xmit(uint8_t rhport, bool force_next) { uint16_t notif_len = sizeof(notify_connected.header) + notify_connected.header.wLength; ncm_epbuf.epnotif = notify_connected; usbd_edpt_xfer(rhport, ncm_interface.ep_notif, (uint8_t *) &ncm_epbuf.epnotif, notif_len, false); + ncm_interface.link_notify_pending = false; // current link state delivered to host ncm_interface.notification_xmit_state = NOTIFICATION_DONE; ncm_interface.notification_xmit_is_running = true; @@ -800,7 +802,48 @@ static void tud_network_recv_renew_r(uint8_t rhport) { } // tud_network_recv_renew /** - * Set the link state and send notification to host + * Flush a pending link-state notification once the notification channel is free. + * + * Runs only in the usbd task context (the deferred link-state setter and the + * notify xfer-completion callback), so all accesses to the notification state + * machine are serialised. It (re)sends a NETWORK_CONNECTION carrying the current + * link state until the host has been told the latest state, so a notification + * that collides with an in-flight one is retried on the next completion instead + * of being silently dropped (which would otherwise leave the host stuck at + * NO-CARRIER after a link-state change). + */ +static void ncm_flush_link_notification(uint8_t rhport) { + if (!ncm_interface.link_notify_pending) { + return; + } + if (ncm_interface.itf_data_alt != 1) { + return; // data interface not active yet; SET_INTERFACE(alt=1) will flush it + } + if (ncm_interface.notification_xmit_is_running) { + return; // channel busy; the in-flight xfer's completion will flush it + } + + // A link toggle does not change the link speed, so only the NETWORK_CONNECTION + // notification needs (re)sending here. + ncm_interface.notification_xmit_state = NOTIFICATION_CONNECTED; + notification_xmit(rhport, true); +} + +/** + * usbd-task trampoline for tud_network_link_state(). + */ +static void ncm_link_notify_task(void *param) { + ncm_flush_link_notification((uint8_t) (uintptr_t) param); +} + +/** + * Set the link state and notify the host. + * + * Records the desired link state as pending and defers the send onto the usbd + * task, so a caller running in a different task than tud_task() cannot race the + * notification state machine against the xfer-completion callback. The pending + * flag guarantees the NETWORK_CONNECTION for the new link state is retried until + * delivered rather than dropped if a previous notification is still in flight. */ void tud_network_link_state(uint8_t rhport, bool is_up) { TU_LOG_DRV("tud_network_link_state(%d, %d)\n", rhport, is_up); @@ -811,18 +854,15 @@ void tud_network_link_state(uint8_t rhport, bool is_up) { } ncm_interface.link_is_up = is_up; + ncm_interface.link_notify_pending = true; - // Only send notification if we have an active data interface + // Only notify if the data interface is active; SET_INTERFACE flushes otherwise. if (ncm_interface.itf_data_alt != 1) { - TU_LOG_DRV(" link state notification skipped (interface not active)\n"); + TU_LOG_DRV(" link state notification deferred (interface not active)\n"); return; } - // Reset notification state to send speed change notification first, then link state notification - ncm_interface.notification_xmit_state = NOTIFICATION_SPEED; - - // Trigger notification transmission - notification_xmit(rhport, false); + usbd_defer_func(ncm_link_notify_task, (void *) (uintptr_t) rhport, false); } //----------------------------------------------------------------------------- @@ -957,6 +997,8 @@ bool netd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint32_ } else if (ep_addr == ncm_interface.ep_notif) { // next transfer on notification channel notification_xmit(rhport, true); + // Channel just freed: (re)send a deferred/collided link-state notification. + ncm_flush_link_notification(rhport); } return true; From 8abd13bd635f4180d27f9d85297147c3fd5ca192 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 a93c144f7e..3052636d8b 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;