Bluetooth: controller: Fix Tx Buffer Overflow
Fix Tx Buffer Overflow caused by uninitialized node_tx memory being used by ULL ISR context due to Compiler Instructions Reordering in the use of MFIFO_ENQUEUE. The MFIFO last index was committed before the data element was stored in the MFIFO due to Compiler Instructions Reordering. This is fixed now by adding a Data Memory Barrier instruction alongwith a compiler memory clobber. Fixes #30378. Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
This commit is contained in:
parent
1a14f8b3a6
commit
1af2b91c23
4 changed files with 15 additions and 7 deletions
|
@ -9,16 +9,18 @@
|
||||||
#include <stddef.h>
|
#include <stddef.h>
|
||||||
|
|
||||||
#include <toolchain.h>
|
#include <toolchain.h>
|
||||||
|
#include <soc.h>
|
||||||
|
|
||||||
#include <sys/util.h>
|
#include <sys/util.h>
|
||||||
|
|
||||||
|
#include "hal/cpu.h"
|
||||||
|
#include "hal/ccm.h"
|
||||||
|
#include "hal/radio.h"
|
||||||
|
|
||||||
#include "util/mem.h"
|
#include "util/mem.h"
|
||||||
#include "util/memq.h"
|
#include "util/memq.h"
|
||||||
#include "util/mfifo.h"
|
#include "util/mfifo.h"
|
||||||
|
|
||||||
#include "hal/ccm.h"
|
|
||||||
#include "hal/radio.h"
|
|
||||||
|
|
||||||
#include "pdu.h"
|
#include "pdu.h"
|
||||||
|
|
||||||
#include "lll.h"
|
#include "lll.h"
|
||||||
|
@ -32,7 +34,6 @@
|
||||||
#define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
|
#define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
|
||||||
#define LOG_MODULE_NAME bt_ctlr_lll_conn
|
#define LOG_MODULE_NAME bt_ctlr_lll_conn
|
||||||
#include "common/log.h"
|
#include "common/log.h"
|
||||||
#include <soc.h>
|
|
||||||
#include "hal/debug.h"
|
#include "hal/debug.h"
|
||||||
|
|
||||||
static int init_reset(void);
|
static int init_reset(void);
|
||||||
|
|
|
@ -6,10 +6,12 @@
|
||||||
|
|
||||||
#include <stddef.h>
|
#include <stddef.h>
|
||||||
#include <zephyr.h>
|
#include <zephyr.h>
|
||||||
|
#include <soc.h>
|
||||||
#include <device.h>
|
#include <device.h>
|
||||||
#include <bluetooth/bluetooth.h>
|
#include <bluetooth/bluetooth.h>
|
||||||
#include <sys/byteorder.h>
|
#include <sys/byteorder.h>
|
||||||
|
|
||||||
|
#include "hal/cpu.h"
|
||||||
#include "hal/ecb.h"
|
#include "hal/ecb.h"
|
||||||
#include "hal/ccm.h"
|
#include "hal/ccm.h"
|
||||||
#include "hal/ticker.h"
|
#include "hal/ticker.h"
|
||||||
|
@ -44,7 +46,6 @@
|
||||||
#define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
|
#define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
|
||||||
#define LOG_MODULE_NAME bt_ctlr_ull_conn
|
#define LOG_MODULE_NAME bt_ctlr_ull_conn
|
||||||
#include "common/log.h"
|
#include "common/log.h"
|
||||||
#include <soc.h>
|
|
||||||
#include "hal/debug.h"
|
#include "hal/debug.h"
|
||||||
|
|
||||||
#if defined(CONFIG_BT_CTLR_USER_EXT)
|
#if defined(CONFIG_BT_CTLR_USER_EXT)
|
||||||
|
|
|
@ -32,9 +32,12 @@
|
||||||
* where A[b] means the A'th link-element, whose mem pointer is b.
|
* where A[b] means the A'th link-element, whose mem pointer is b.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
#include <zephyr/types.h>
|
|
||||||
#include <stddef.h>
|
#include <stddef.h>
|
||||||
|
|
||||||
|
#include <soc.h>
|
||||||
|
|
||||||
|
#include "hal/cpu.h"
|
||||||
|
|
||||||
#include "memq.h"
|
#include "memq.h"
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -97,7 +100,8 @@ memq_link_t *memq_enqueue(memq_link_t *link, void *mem, memq_link_t **tail)
|
||||||
/* Update the tail-pointer to point to the new tail element.
|
/* Update the tail-pointer to point to the new tail element.
|
||||||
* The new tail-element is not expected to point to anything sensible
|
* The new tail-element is not expected to point to anything sensible
|
||||||
*/
|
*/
|
||||||
*tail = link;
|
cpu_dmb(); /* Ensure data accesses are synchronized */
|
||||||
|
*tail = link; /* Commit: enqueue of memq node */
|
||||||
|
|
||||||
return link;
|
return link;
|
||||||
}
|
}
|
||||||
|
|
|
@ -123,6 +123,7 @@ static inline void mfifo_by_idx_enqueue(uint8_t *fifo, uint8_t size, uint8_t idx
|
||||||
void **p = (void **)(fifo + (*last) * size); /* buffer preceding idx */
|
void **p = (void **)(fifo + (*last) * size); /* buffer preceding idx */
|
||||||
*p = mem; /* store the payload which for API 2 is only a void-ptr */
|
*p = mem; /* store the payload which for API 2 is only a void-ptr */
|
||||||
|
|
||||||
|
cpu_dmb(); /* Ensure data accesses are synchronized */
|
||||||
*last = idx; /* Commit: Update write index */
|
*last = idx; /* Commit: Update write index */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -189,6 +190,7 @@ static inline uint8_t mfifo_enqueue_get(uint8_t *fifo, uint8_t size, uint8_t cou
|
||||||
*/
|
*/
|
||||||
static inline void mfifo_enqueue(uint8_t idx, uint8_t *last)
|
static inline void mfifo_enqueue(uint8_t idx, uint8_t *last)
|
||||||
{
|
{
|
||||||
|
cpu_dmb(); /* Ensure data accesses are synchronized */
|
||||||
*last = idx; /* Commit: Update write index */
|
*last = idx; /* Commit: Update write index */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue