Skip to content

Hard fault: NULL-pointer dereference in teseo_rx_callback_error() (UART mode) #4

Description

@dtiziano

Summary

teseo_rx_callback_error() in Drivers/BSP/Components/teseo_liv3f/teseo_liv3f_uart.c
unconditionally dereferences Teseo_UART_Data.wr_msg:

static void teseo_rx_callback_error(void)
{
  const TESEO_LIV3F_ctx_t *pCtx = Teseo_UART_Data.pCtx;

  PRINT_DBG("E");

  Teseo_UART_Data.fsm_next_state = fsm_discard;
  Teseo_UART_Data.wr_msg->len = 0;          /* <-- wr_msg may be NULL */
  pCtx->ClearOREF(pCtx->Handle);
  //__HAL_UART_CLEAR_FLAG(teseo_io_data->huart, UART_FLAG_ORE);
  pCtx->Receive(pCtx->Handle, &Teseo_UART_Data.dummy_char, 1);
}

However, the driver's own FSM sets wr_msg to NULL in two places in the same file:

  1. teseo_uart_rx_onoff() — before reception is (re)started:
    Teseo_UART_Data.wr_msg = NULL;
    Teseo_UART_Data.fsm_state = fsm_discard;
    Teseo_UART_Data.fsm_next_state = fsm_discard;
  2. teseo_rx_callback_ok() — after every completed NMEA sentence
    (fsm_synch case, on receiving the terminating '$'):
    teseo_queue_release_wr_buffer(pCtx->pQueue, Teseo_UART_Data.wr_msg);
    Teseo_UART_Data.wr_msg = NULL;
    Teseo_UART_Data.fsm_next_state = fsm_discard;

wr_msg is only assigned again once the FSM, in discard mode, receives the next
'$' and claims a buffer (teseo_queue_claim_wr_buffer).

Consequently, any UART error (e.g. overrun) that occurs while the FSM is in
discard mode
— i.e. before the first sentence sync, and in the window between
every two sentences — executes Teseo_UART_Data.wr_msg->len = 0; with
wr_msg == NULL. Because the callback runs in UART interrupt context, this is a
write to address 0x00000004 from an ISR, which raises a BusFault that
escalates to a HardFault.

Environment

Reproduction / trigger conditions

The error callback fires on UART overrun. With the driver's 1-byte-at-a-time
HAL_UART_Receive_IT scheme, an overrun happens whenever interrupt latency
exceeds ~2 byte times. In a minimal example this is rare, which is presumably
why the bug goes unnoticed; in a realistic application (SDMMC writes, other
ISRs, RTOS critical sections) it is easy to hit.

Deterministic scenario observed on our hardware:

  1. Cold power-on with the Teseo-LIV3F powered from the same rail as the MCU.
  2. The Teseo boots in parallel and immediately transmits its boot output while
    the host is still initializing peripherals (elevated interrupt latency).
  3. An overrun occurs while the FSM is still in discard mode (wr_msg == NULL,
    as set by teseo_uart_rx_onoff).
  4. Hard fault.

A warm MCU reset (Teseo already running and streaming steadily) rarely
triggers it, because the FSM syncs within a few bytes and errors tend to land
while a buffer is claimed — which makes the failure look intermittent and
power-cycle-related.

Evidence

Disassembly of the faulting build (arm-none-eabi-gcc, -O0):

156         Teseo_UART_Data.wr_msg->len = 0;
0800da7e:   ldr  r3, [pc, #40]   ; r3 = &Teseo_UART_Data
0800da80:   ldr  r3, [r3, #4]    ; r3 = Teseo_UART_Data.wr_msg  (== NULL)
0800da82:   movs r2, #0
0800da84:   strh r2, [r3, #4]    ; write to 0x00000004 -> BusFault -> HardFault

The stacked PC in the hard fault handler points at the strh (0x0800DA84).

Suggested fix

Guard the dereference (and, defensively, pCtx). Recovery behavior is
unchanged: the FSM is reset to discard mode either way, and
teseo_queue_claim_wr_buffer() resets len = 0 when the next buffer is
claimed, so skipping the reset for a NULL wr_msg loses nothing.

static void teseo_rx_callback_error(void)
{
  const TESEO_LIV3F_ctx_t *pCtx = Teseo_UART_Data.pCtx;

  PRINT_DBG("E");

  Teseo_UART_Data.fsm_next_state = fsm_discard;
  /* wr_msg is NULL while the FSM is in discard mode (set NULL in
   * teseo_uart_rx_onoff() and after every completed sentence in
   * teseo_rx_callback_ok()) */
  if (Teseo_UART_Data.wr_msg != NULL)
  {
    Teseo_UART_Data.wr_msg->len = 0;
  }
  if (pCtx != NULL)
  {
    pCtx->ClearOREF(pCtx->Handle);
    pCtx->Receive(pCtx->Handle, &Teseo_UART_Data.dummy_char, 1);
  }
}

We have been running with this patch; the cold power-on scenario now recovers
cleanly (the FSM resynchronizes on the next '$').

Related observation (minor)

PRINT_DBG is invoked from this interrupt context. If an application maps
PRINT_OUT/PRINT_DBG (via teseo_liv3f_conf.h) to printf-based logging,
newlib's first printf call allocates its stream buffer with malloc, which
is not safe in an ISR. It may be worth documenting that the PRINT_* hooks can
be called from interrupt context and must be ISR-safe.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions