zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.52k stars 6.45k forks source link

Race Condition in uarte_nrfx_rx_disable Function #77060

Open bfa8 opened 1 month ago

bfa8 commented 1 month ago

Description of the Bug The function uarte_nrfx_rx_disable in /sdk-zephyr/drivers/serial/uart_nrfx_uarte.c assigns values to certain flags that are used to disable the RX in the ISR. However, uarte_nrfx_rx_disable is not safe when the RX is disabled in parallel within the ISR.

An edge case was encountered where, just before the application thread called uarte_nrfx_rx_disable, a UART framing error triggered an interrupt, which called uarte_nrfx_rx_disable in the ISR. During the execution of uarte_nrfx_rx_disable in the application thread - just before the RX disabling flags were set - the thread was interrupted by the ISR that disable the RX due to the UART framing error.

After returning to the application thread execution, the flags in uarte_nrfx_rx_disable were set again, but the ISR to disable RX was never happen. Consequently, it became impossible to re-enable RX due to the flags set in uarte_nrfx_rx_disable. The driver still waits for disable RX, even though it has already happened.

To Reproduce

  1. Develop an application that experiences frequent UART framing errors.
  2. Continuously start and stop reading data from the UART.

Proposed Solution To prevent uarte_nrfx_rx_disable from being interrupted during its execution, interrupt locks should be added inside the function. This will ensure that the function cannot be preempted by an ISR while it is in progress.

Here’s how you could modify the function:

static int uarte_nrfx_rx_disable(const struct device *dev)
{
    struct uarte_nrfx_data *data = dev->data;
    NRF_UARTE_Type *uarte = get_uarte_instance(dev);
    unsigned int key = irq_lock();
    if (data->async->rx_buf == NULL) {
        irq_unlock(key);
        return -EFAULT;
    }
    if (data->async->rx_next_buf != NULL) {
        nrf_uarte_shorts_disable(uarte, NRF_UARTE_SHORT_ENDRX_STARTRX);
        nrf_uarte_event_clear(uarte, NRF_UARTE_EVENT_RXSTARTED);
    }
    k_timer_stop(&data->async->rx_timeout_timer);
    data->async->rx_enabled = false;
    data->async->discard_rx_fifo = true;
    nrf_uarte_task_trigger(uarte, NRF_UARTE_TASK_STOPRX);

    irq_unlock(key);
    return 0;
}

Impact RX can no longer be activated.

Environment OS: Linux Toolchain: Zephyr SDK 0.16.3 Nordic Zephyr Version: 3.4.99-ncs1-1 (SHA 83980fe1679441be9b0e1db556a353f6118fe14f)

nordic-piks commented 1 week ago

Triaged internally