fix(usb-ncm): NULL deref race in xmit_setup_next_glue_ntb + osal_spin_lock protection (T-145)#4
Open
chipsoft wants to merge 2 commits into
Open
fix(usb-ncm): NULL deref race in xmit_setup_next_glue_ntb + osal_spin_lock protection (T-145)#4chipsoft wants to merge 2 commits into
chipsoft wants to merge 2 commits into
Conversation
…nd tud_network_xmit Both functions read ncm_interface.xmit_glue_ntb twice: once for a NULL check, then again to assign a local pointer. A concurrent SET_INTERFACE or DMA-done callback (USB task) can zero xmit_glue_ntb between the two reads via ncm_flush_data_paths() or xmit_start_if_possible(), causing the second read to return NULL and the subsequent field write to address 0. Crash site: ncm_device.c:509, pc=0x08101690, r3=0 → SecureFault → TF-M SYSRESETREQ → nboot ISP loop (T-145). Fix: capture the pointer once into a local variable, NULL-check the local, and publish to the global only after the NTB is fully initialised (xmit_setup_next_glue_ntb). In tud_network_xmit, snapshot the global before the NULL check so the same pointer is used throughout. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tate machine The lwIP task (tud_network_can_xmit / tud_network_xmit) and the USB task (tud_task → ncm_flush_data_paths / xmit_start_if_possible) both mutate ncm_interface.xmit_glue_ntb and xmit_glue_ntb_datagram_ndx without synchronisation, creating data races on a preemptive FreeRTOS scheduler. The T-145.1 local-variable fix prevents the immediate NULL-deref crash; this commit closes the residual race window completely. Protected regions (single file-static osal_spinlock_t s_xmit_glue_lock): ncm_flush_data_paths() — zero xmit_glue_ntb + datagram_ndx (USB task) xmit_start_if_possible() — guard check + steal of xmit_glue_ntb (USB task) xmit_setup_next_glue_ntb() — snapshot old ptr, publish new ptr + ndx (lwIP task) tud_network_xmit() — snapshot ptr, reserve datagram slot (lwIP task) RW612 is single-core (TUP_MCU_MULTIPLE_CORE=0); osal_spin_lock(false) maps to taskENTER_CRITICAL() which masks interrupts up to configMAX_SYSCALL_ INTERRUPT_PRIORITY — includes the USB IRQ at configLIBRARY_LOWEST_ INTERRUPT_PRIORITY-1. All protected sections are short and non-blocking. Co-Authored-By: Claude Sonnet 4.6 <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
1 participant
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.
Summary
xmit_setup_next_glue_ntbandtud_network_xmit: capturencm_interface.xmit_glue_ntbinto a local once, NULL-check and use the local, publish to global only after full NTB init.osal_spin_lockcritical sections (→taskENTER_CRITICALon single-core RW612) to all four concurrentxmit_glue_ntb/xmit_glue_ntb_datagram_ndxmutation points:xmit_setup_next_glue_ntb,tud_network_xmit,xmit_start_if_possible,ncm_flush_data_paths.Root cause
lwIP task and USB task (
tud_task→ncm_flush_data_pathsor DMA-done completion) concurrently accessncm_interface.xmit_glue_ntbwithout synchronisation. The lwIP task checked the pointer, the USB task zeroed it, the lwIP task dereferenced it → write to address 0 → NS SecureFault → TF-M SYSRESETREQ → nboot ISP loop.Crash:
pc=0x08101690(ncm_device.c:509),r3=0in exception frame.Test plan
CAN1_OVER_FLEXCOMM0=1 DISABLE_ROLLBACK=1 MCUBOOT_IMG_VERSION=3.6.0 ./scripts/verify.sh mcuboot→ PASS (build + flash + RTT ALIVE)🤖 Generated with Claude Code