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>

Author: cvinayak

subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c

@@ -9,16 +9,18 @@
 #include <stddef.h>
 
 #include <toolchain.h>
+#include <soc.h>
 
 #include <sys/util.h>
 
+#include "hal/cpu.h"
+#include "hal/ccm.h"
+#include "hal/radio.h"
+
 #include "util/mem.h"
 #include "util/memq.h"
 #include "util/mfifo.h"
 
-#include "hal/ccm.h"
-#include "hal/radio.h"
-
 #include "pdu.h"
 
 #include "lll.h"
@@ -32,7 +34,6 @@
 #define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
 #define LOG_MODULE_NAME bt_ctlr_lll_conn
 #include "common/log.h"
-#include <soc.h>
 #include "hal/debug.h"
 
 static int init_reset(void);

subsys/bluetooth/controller/ll_sw/ull_conn.c

@@ -6,10 +6,12 @@
 
 #include <stddef.h>
 #include <zephyr.h>
+#include <soc.h>
 #include <device.h>
 #include <bluetooth/bluetooth.h>
 #include <sys/byteorder.h>
 
+#include "hal/cpu.h"
 #include "hal/ecb.h"
 #include "hal/ccm.h"
 #include "hal/ticker.h"
@@ -44,7 +46,6 @@
 #define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER)
 #define LOG_MODULE_NAME bt_ctlr_ull_conn
 #include "common/log.h"
-#include <soc.h>
 #include "hal/debug.h"
 
 #if defined(CONFIG_BT_CTLR_USER_EXT)

subsys/bluetooth/controller/util/memq.c

@@ -32,9 +32,12 @@
  *   where A[b] means the A'th link-element, whose mem pointer is b.
  */
 
-#include <zephyr/types.h>
 #include <stddef.h>
 
+#include <soc.h>
+
+#include "hal/cpu.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.
 	 * 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;
 }

subsys/bluetooth/controller/util/mfifo.h

@@ -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 */
 	*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 */
 }
 
@@ -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)
 {
+	cpu_dmb(); /* Ensure data accesses are synchronized */
 	*last = idx; /* Commit: Update write index */
 }