Bluetooth: controller: fixing rx node leak on CPR reject of parallel CPR

In case a CPR is intiated but rejected due to CPR active on
other connection, rx nodes are leaked due to retained node not
being properly released.

Signed-off-by: Erik Brockhoff <[email protected]>
(cherry picked from commit edef1b7)
Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>

Author: erbr-ot

subsys/bluetooth/controller/ll_sw/ull_llcp.c

@@ -291,17 +291,30 @@ void llcp_rx_node_retain(struct proc_ctx *ctx)
 	ctx->node_ref.rx->hdr.link = ctx->node_ref.link;
 }
 
+void llcp_rx_node_release(struct proc_ctx *ctx)
+{
+	LL_ASSERT(ctx->node_ref.rx);
+
+	if (ctx->node_ref.rx->hdr.type == NODE_RX_TYPE_RETAIN) {
+		/* Mark RX node to release and release */
+		ctx->node_ref.rx->hdr.type = NODE_RX_TYPE_RELEASE;
+		ll_rx_put_sched(ctx->node_ref.rx->hdr.link, ctx->node_ref.rx);
+	}
+}
+
 void llcp_nodes_release(struct ll_conn *conn, struct proc_ctx *ctx)
 {
 	if (ctx->node_ref.rx && ctx->node_ref.rx->hdr.type == NODE_RX_TYPE_RETAIN) {
 		/* RX node retained, so release */
 		ctx->node_ref.rx->hdr.link->mem = conn->llcp.rx_node_release;
+		ctx->node_ref.rx->hdr.type = NODE_RX_TYPE_RELEASE;
 		conn->llcp.rx_node_release = ctx->node_ref.rx;
 	}
 #if defined(CONFIG_BT_CTLR_PHY) && defined(CONFIG_BT_CTLR_DATA_LENGTH)
 	if (ctx->proc == PROC_PHY_UPDATE && ctx->data.pu.ntf_dle_node) {
 		/* RX node retained, so release */
 		ctx->data.pu.ntf_dle_node->hdr.link->mem = conn->llcp.rx_node_release;
+		ctx->data.pu.ntf_dle_node->hdr.type = NODE_RX_TYPE_RELEASE;
 		conn->llcp.rx_node_release = ctx->data.pu.ntf_dle_node;
 	}
 #endif
@@ -706,9 +719,6 @@ void ull_cp_release_nodes(struct ll_conn *conn)
 		hdr = &rx->hdr;
 		rx = hdr->link->mem;
 
-		/* Mark for buffer for release */
-		hdr->type = NODE_RX_TYPE_RELEASE;
-
 		/* enqueue rx node towards Thread */
 		ll_rx_put(hdr->link, hdr);
 	}

subsys/bluetooth/controller/ll_sw/ull_llcp_common.c

@@ -1139,6 +1139,7 @@ static void rp_comm_ntf(struct ll_conn *conn, struct proc_ctx *ctx, uint8_t gene
 
 	/* Allocate ntf node */
 	ntf = ctx->node_ref.rx;
+	ctx->node_ref.rx = NULL;
 	LL_ASSERT(ntf);
 
 	/* This should be an 'old' RX node, so put/sched when done */

subsys/bluetooth/controller/ll_sw/ull_llcp_conn_upd.c

@@ -633,8 +633,7 @@ static void lp_cu_check_instant(struct ll_conn *conn, struct proc_ctx *ctx, uint
 			lp_cu_ntf_complete(conn, ctx, evt, param);
 		} else {
 			/* Release RX node kept for NTF */
-			ctx->node_ref.rx->hdr.type = NODE_RX_TYPE_RELEASE;
-			ll_rx_put_sched(ctx->node_ref.rx->hdr.link, ctx->node_ref.rx);
+			llcp_rx_node_release(ctx);
 			ctx->node_ref.rx = NULL;
 
 			lp_cu_complete(conn, ctx);
@@ -973,11 +972,18 @@ static void rp_cu_st_wait_conn_param_req_available(struct ll_conn *conn, struct
 	case RP_CU_EVT_RUN:
 		if (cpr_active_is_set(conn)) {
 			ctx->state = RP_CU_STATE_WAIT_CONN_PARAM_REQ_AVAILABLE;
+
 			if (!llcp_rr_ispaused(conn) && llcp_tx_alloc_peek(conn, ctx)) {
 				/* We're good to reject immediately */
 				ctx->data.cu.rejected_opcode = PDU_DATA_LLCTRL_TYPE_CONN_PARAM_REQ;
 				ctx->data.cu.error = BT_HCI_ERR_UNSUPP_LL_PARAM_VAL;
 				rp_cu_send_reject_ext_ind(conn, ctx, evt, param);
+
+				/* Possibly retained rx node to be released as we won't need it */
+				llcp_rx_node_release(ctx);
+				ctx->node_ref.rx = NULL;
+
+				break;
 			}
 			/* In case we have to defer NTF */
 			llcp_rx_node_retain(ctx);
@@ -992,6 +998,9 @@ static void rp_cu_st_wait_conn_param_req_available(struct ll_conn *conn, struct
 				rp_cu_conn_param_req_ntf(conn, ctx);
 				ctx->state = RP_CU_STATE_WAIT_CONN_PARAM_REQ_REPLY;
 			} else {
+				/* Possibly retained rx node to be released as we won't need it */
+				llcp_rx_node_release(ctx);
+				ctx->node_ref.rx = NULL;
 #if defined(CONFIG_BT_CTLR_USER_CPR_ANCHOR_POINT_MOVE)
 				/* Handle APM as a vendor specific user extension */
 				if (conn->lll.role == BT_HCI_ROLE_PERIPHERAL &&
@@ -1177,8 +1186,7 @@ static void rp_cu_check_instant(struct ll_conn *conn, struct proc_ctx *ctx, uint
 			cu_ntf(conn, ctx);
 		} else {
 			/* Release RX node kept for NTF */
-			ctx->node_ref.rx->hdr.type = NODE_RX_TYPE_RELEASE;
-			ll_rx_put_sched(ctx->node_ref.rx->hdr.link, ctx->node_ref.rx);
+			llcp_rx_node_release(ctx);
 			ctx->node_ref.rx = NULL;
 		}
 		rp_cu_complete(conn, ctx);

subsys/bluetooth/controller/ll_sw/ull_llcp_enc.c

@@ -225,6 +225,7 @@ static void lp_enc_ntf(struct ll_conn *conn, struct proc_ctx *ctx)
 
 	/* Piggy-back on RX node */
 	ntf = ctx->node_ref.rx;
+	ctx->node_ref.rx = NULL;
 	LL_ASSERT(ntf);
 
 	ntf->hdr.type = NODE_RX_TYPE_DC_PDU;

subsys/bluetooth/controller/ll_sw/ull_llcp_internal.h

@@ -413,6 +413,7 @@ void llcp_ntf_set_pending(struct ll_conn *conn);
 void llcp_ntf_clear_pending(struct ll_conn *conn);
 bool llcp_ntf_pending(struct ll_conn *conn);
 void llcp_rx_node_retain(struct proc_ctx *ctx);
+void llcp_rx_node_release(struct proc_ctx *ctx);
 
 /*
  * ULL -> LLL Interface

subsys/bluetooth/controller/ll_sw/ull_llcp_local.c

@@ -81,6 +81,12 @@ void llcp_lr_check_done(struct ll_conn *conn, struct proc_ctx *ctx)
 		ctx_header = llcp_lr_peek(conn);
 		LL_ASSERT(ctx_header == ctx);
 
+		/* If we have a node rx it must not be marked RETAIN as
+		 * the memory referenced would leak
+		 */
+		LL_ASSERT(ctx->node_ref.rx == NULL ||
+			  ctx->node_ref.rx->hdr.type != NODE_RX_TYPE_RETAIN);
+
 		lr_dequeue(conn);
 
 		llcp_proc_ctx_release(ctx);

subsys/bluetooth/controller/ll_sw/ull_llcp_phy.c

@@ -433,6 +433,7 @@ static void pu_ntf(struct ll_conn *conn, struct proc_ctx *ctx)
 
 	/* Piggy-back on stored RX node */
 	ntf = ctx->node_ref.rx;
+	ctx->node_ref.rx = NULL;
 	LL_ASSERT(ntf);
 
 	if (ctx->data.pu.ntf_pu) {

subsys/bluetooth/controller/ll_sw/ull_llcp_remote.c

@@ -118,6 +118,12 @@ void llcp_rr_check_done(struct ll_conn *conn, struct proc_ctx *ctx)
 		ctx_header = llcp_rr_peek(conn);
 		LL_ASSERT(ctx_header == ctx);
 
+		/* If we have a node rx it must not be marked RETAIN as
+		 * the memory referenced would leak
+		 */
+		LL_ASSERT(ctx->node_ref.rx == NULL ||
+			  ctx->node_ref.rx->hdr.type != NODE_RX_TYPE_RETAIN);
+
 		rr_dequeue(conn);
 
 		llcp_proc_ctx_release(ctx);