NULL dereference, TinyUSB thread & ISR shared variables strategy #2812
bencowperthwaite
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
I have observed a NULL pointer dereference with TinyUSB which has prompted a wider question (in my mind) about how TinyUSB handles the access of shared structures/variables from both ISRs and threads.
I have a dual-lun MSC set up on an STM32U595 (Synopsis DWC2).
I'm using TinyUSB v0.16.0.
I'm using FreeRTOS, but I have a custom wrapper around the kernel, so I'm using OPT_OS_CUSTOM.
First, the issue at hand:
I have caught NULL pointer dereference inside_ write_fifo_packet() and read_fifo_packet() in dcd_dwc2.c
A xfer_ctl_t->buffer is passed as src and dst respectively and is accessed without a NULL check.
The good news is this doesn't happen (as far as I have observed) in "normal" operation, I catch it when I'm spending time stopped at breakpoints.
The issue (as far as I can tell) is the buffers in question belongs to static xfer_ctl_t xfer_status[DWC2_EP_MAX][2]; (line 92 dcd_dwc2.c), and these static structs are accessed by both threaded context inside tud_task() (in dcd_edpt_xfer() for example) and inside an ISR context in handle_epin_irq() and handle_rxflvl_irq().
Clearly we are expecting the buffer to be valid (and not-NULL) inside the ISR, however in certain situations (when stopped due to a breakpoint) the tud_task() might not have had the chance to set it before the ISR is executed.
Now clearly spending long amounts of time in breakpoints is not normal operation, but it does expose a race hazard.
Without some other mitigation, checks or guards, one cannot guarantee that any given task will have performed a certain amount of processing by a particular point in time.
Periods of high processing demand on higher priority tasks can easily recreate the same conditions as a breakpoint.
Or am I using TinyUSB wrong? I am presuming that the USB hardware interrupts should be higher priority than tud_task(), but is this wrong?
Beta Was this translation helpful? Give feedback.
All reactions