From 88d604d88c17130ec94c29f9011b0a9d11359c8d Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 18 Feb 2021 10:38:11 -0600 Subject: [PATCH] drv/bluetooth/cc2640: protect against write_buf use while reading Due to the use of write_buf_size as a flag to indicate if write_buf is currently in use, there was a possibility that the UART service or Pybricks service could write to write_buf and set write_buf_size while a read-only SPI transaction was in progress, which would cause data to be lost. --- lib/ble5stack/central/hci_tl.h | 4 ++-- .../drv/bluetooth/bluetooth_stm32_cc2640.c | 23 +++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/lib/ble5stack/central/hci_tl.h b/lib/ble5stack/central/hci_tl.h index 0a68dca8b..f7ce08281 100755 --- a/lib/ble5stack/central/hci_tl.h +++ b/lib/ble5stack/central/hci_tl.h @@ -38,8 +38,8 @@ #include /* HCI Buffer size */ -#define TX_BUFFER_SIZE 256 -#define RX_BUFFER_SIZE 256 +#define TX_BUFFER_SIZE 255 +#define RX_BUFFER_SIZE 255 #define HCI_FRAME_SIZE 0x04 diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c index c2e78c82d..8f03e6797 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c @@ -1184,18 +1184,29 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { start_task(init_task, NULL); while (true) { + static uint8_t real_write_xfer_size; + PROCESS_WAIT_UNTIL(spi_srdy || write_xfer_size); spi_set_mrdy(true); + // Need to make a copy of the write transfer size for the case where we + // are reading only. In this case we still have to set write_xfer_size + // to a non-zero value to indicate that write_buf is being used. But we + // still need the real size later to figure out the actual size of the + // SPI transfer. + real_write_xfer_size = write_xfer_size; + // we can either read, write, or read and write at the same time - if (write_xfer_size) { + if (real_write_xfer_size) { // if we are writing only, we have to wait until SRDY is asserted PROCESS_WAIT_UNTIL(spi_srdy); } else { // if we are reading only, the write buffer has to be all 0s memset(write_buf, 0, PBIO_ARRAY_SIZE(write_buf)); + // indicates that write_buf is in use + write_xfer_size = PBIO_ARRAY_SIZE(write_buf); } // send the write header @@ -1205,10 +1216,10 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { // Total transfer size is biggest of read and write sizes. read_xfer_size = 0; - if (get_npi_rx_size(&read_xfer_size) && read_xfer_size > write_xfer_size - 4) { + if (get_npi_rx_size(&read_xfer_size) && read_xfer_size > real_write_xfer_size - 4) { xfer_size = read_xfer_size + 4; } else { - xfer_size = write_xfer_size; + xfer_size = real_write_xfer_size; } // read the remaining message @@ -1220,10 +1231,8 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) { spi_set_mrdy(false); PROCESS_WAIT_UNTIL(!spi_srdy); - if (write_xfer_size) { - // set to 0 to indicate that xfer is complete - write_xfer_size = 0; - } + // set to 0 to indicate that xfer is complete + write_xfer_size = 0; if (read_xfer_size) { // handle the received data