From 1af2b91c23629a1ec958b0723726bae656a20de7 Mon Sep 17 00:00:00 2001
From: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Date: Mon, 1 Feb 2021 07:34:29 +0530
Subject: [PATCH] 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>
---
 subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c | 9 +++++----
 subsys/bluetooth/controller/ll_sw/ull_conn.c            | 3 ++-
 subsys/bluetooth/controller/util/memq.c                 | 8 ++++++--
 subsys/bluetooth/controller/util/mfifo.h                | 2 ++
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c b/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c
index f8ea233dde9..a9af12dcea3 100644
--- a/subsys/bluetooth/controller/ll_sw/nordic/lll/lll_conn.c
+++ b/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);
diff --git a/subsys/bluetooth/controller/ll_sw/ull_conn.c b/subsys/bluetooth/controller/ll_sw/ull_conn.c
index cb650279960..8489b60b51d 100644
--- a/subsys/bluetooth/controller/ll_sw/ull_conn.c
+++ b/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)
diff --git a/subsys/bluetooth/controller/util/memq.c b/subsys/bluetooth/controller/util/memq.c
index f446b1fbda0..da4edfe677e 100644
--- a/subsys/bluetooth/controller/util/memq.c
+++ b/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;
 }
diff --git a/subsys/bluetooth/controller/util/mfifo.h b/subsys/bluetooth/controller/util/mfifo.h
index 19695933d8a..a4616f426e5 100644
--- a/subsys/bluetooth/controller/util/mfifo.h
+++ b/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 */
 }