Skip to content

Commit c3f1075

Browse files
cvinayakkartben
authored andcommitted
Bluetooth: Controller: Add ll_conn_get() return value check
Add ll_conn_get() return value check for valid connection context. Build command: cmake -GNinja -DBOARD=nrf52833dk/nrf52833 -DEXTRA_CONF_FILE=overlay-all-bt_ll_sw_split.conf -DDTC_OVERLAY_FILE=boards/nrf52833dk_nrf52833_df.overlay -DSNIPPET="bt-ll-sw-split" ../../samples/bluetooth/hci_uart ninja Before: Memory region Used Size Region Size %age Used FLASH: 283716 B 512 KB 54.11% RAM: 109752 B 128 KB 83.73% IDT_LIST: 0 GB 32 KB 0.00% After: Memory region Used Size Region Size %age Used FLASH: 284992 B 512 KB 54.36% RAM: 109752 B 128 KB 83.73% IDT_LIST: 0 GB 32 KB 0.00% After (use of `conn != NULL`): Memory region Used Size Region Size %age Used FLASH: 285044 B 512 KB 54.37% RAM: 109752 B 128 KB 83.73% IDT_LIST: 0 GB 32 KB 0.00% Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
1 parent a396bdd commit c3f1075

File tree

8 files changed

+70
-9
lines changed

8 files changed

+70
-9
lines changed

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ static int prepare_cb(struct lll_prepare_param *p)
155155

156156
/* Get reference to ACL context */
157157
conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
158+
LL_ASSERT(conn_lll != NULL);
158159

159160
/* Pick the event_count calculated in the ULL prepare */
160161
cis_lll->event_count = cis_lll->event_count_prepare;
@@ -433,6 +434,8 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
433434
/* Get reference to ACL context */
434435
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
435436

437+
LL_ASSERT(conn_lll != NULL);
438+
436439
if (conn_lll->enc_rx) {
437440
radio_ccm_disable();
438441
}
@@ -479,6 +482,8 @@ static void isr_tx(void *param)
479482
#if defined(CONFIG_BT_CTLR_LE_ENC)
480483
/* Get reference to ACL context */
481484
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
485+
486+
LL_ASSERT(conn_lll != NULL);
482487
#endif /* CONFIG_BT_CTLR_LE_ENC */
483488

484489
/* PHY */
@@ -584,6 +589,7 @@ static void isr_tx(void *param)
584589

585590
/* Get reference to ACL context */
586591
evt_conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
592+
LL_ASSERT(evt_conn_lll != NULL);
587593

588594
/* Calculate the radio channel to use for next subevent */
589595
data_chan_id = lll_chan_id(cis_lll->access_addr);
@@ -630,6 +636,7 @@ static void isr_tx(void *param)
630636

631637
/* Get reference to ACL context */
632638
next_conn_lll = ull_conn_lll_get(next_cis_lll->acl_handle);
639+
LL_ASSERT(next_conn_lll != NULL);
633640

634641
/* Calculate the radio channel to use for ISO event */
635642
data_chan_id = lll_chan_id(next_cis_lll->access_addr);
@@ -770,6 +777,8 @@ static void isr_rx(void *param)
770777
/* Get reference to ACL context */
771778
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
772779

780+
LL_ASSERT(conn_lll != NULL);
781+
773782
/* If required, wait for CCM to finish
774783
*/
775784
if (pdu_rx->len && conn_lll->enc_rx) {
@@ -857,6 +866,7 @@ static void isr_rx(void *param)
857866

858867
/* Get reference to ACL context */
859868
next_conn_lll = ull_conn_lll_get(next_cis_lll->acl_handle);
869+
LL_ASSERT(next_conn_lll != NULL);
860870

861871
/* Calculate CIS channel if not already calculated */
862872
if (se_curr < cis_lll->nse) {
@@ -1032,6 +1042,8 @@ static void isr_prepare_subevent(void *param)
10321042
#if defined(CONFIG_BT_CTLR_LE_ENC)
10331043
/* Get reference to ACL context */
10341044
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
1045+
1046+
LL_ASSERT(conn_lll != NULL);
10351047
#endif /* CONFIG_BT_CTLR_LE_ENC */
10361048

10371049
/* PHY */

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ static int prepare_cb(struct lll_prepare_param *p)
169169

170170
/* Get reference to ACL context */
171171
conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
172+
LL_ASSERT(conn_lll != NULL);
172173

173174
/* Pick the event_count calculated in the ULL prepare */
174175
cis_lll->event_count = cis_lll->event_count_prepare;
@@ -449,6 +450,8 @@ static void abort_cb(struct lll_prepare_param *prepare_param, void *param)
449450
/* Get reference to ACL context */
450451
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
451452

453+
LL_ASSERT(conn_lll != NULL);
454+
452455
if (conn_lll->enc_rx) {
453456
radio_ccm_disable();
454457
}
@@ -563,6 +566,7 @@ static void isr_rx(void *param)
563566

564567
/* Get reference to ACL context */
565568
conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
569+
LL_ASSERT(conn_lll != NULL);
566570

567571
if (crc_ok) {
568572
struct node_rx_pdu *node_rx;
@@ -876,6 +880,8 @@ static void isr_tx(void *param)
876880
#if defined(CONFIG_BT_CTLR_LE_ENC)
877881
/* Get reference to ACL context */
878882
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
883+
884+
LL_ASSERT(conn_lll != NULL);
879885
#endif /* CONFIG_BT_CTLR_LE_ENC */
880886

881887
/* PHY */
@@ -1040,6 +1046,7 @@ static void isr_prepare_subevent(void *param)
10401046

10411047
/* Get reference to ACL context */
10421048
conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
1049+
LL_ASSERT(conn_lll != NULL);
10431050

10441051
/* Calculate the radio channel to use for next subevent
10451052
*/
@@ -1065,6 +1072,7 @@ static void isr_prepare_subevent_next_cis(void *param)
10651072

10661073
/* Get reference to ACL context */
10671074
conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
1075+
LL_ASSERT(conn_lll != NULL);
10681076

10691077
/* Event counter value, 0-15 bit of cisEventCounter */
10701078
event_counter = cis_lll->event_count;
@@ -1101,6 +1109,8 @@ static void isr_prepare_subevent_common(void *param)
11011109
#if defined(CONFIG_BT_CTLR_LE_ENC)
11021110
/* Get reference to ACL context */
11031111
const struct lll_conn *conn_lll = ull_conn_lll_get(cis_lll->acl_handle);
1112+
1113+
LL_ASSERT(conn_lll != NULL);
11041114
#endif /* CONFIG_BT_CTLR_LE_ENC */
11051115

11061116
/* PHY */

subsys/bluetooth/controller/ll_sw/ull.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,6 +1686,7 @@ void ll_rx_mem_release(void **node_rx)
16861686
memq_link_t *link;
16871687

16881688
conn = ll_conn_get(rx_free->hdr.handle);
1689+
LL_ASSERT(conn != NULL);
16891690

16901691
LL_ASSERT(!conn->lll.link_tx_free);
16911692
link = memq_deinit(&conn->lll.memq_tx.head,
@@ -2868,6 +2869,8 @@ static inline void rx_demux_rx(memq_link_t *link, struct node_rx_hdr *rx)
28682869
(void)memq_dequeue(memq_ull_rx.tail, &memq_ull_rx.head, NULL);
28692870

28702871
conn = ll_conn_get(rx->handle);
2872+
LL_ASSERT(conn != NULL);
2873+
28712874
if (ull_cp_cc_awaiting_established(conn)) {
28722875
ull_cp_cc_established(conn, BT_HCI_ERR_SUCCESS);
28732876
}

subsys/bluetooth/controller/ll_sw/ull_central_iso.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,7 @@ uint8_t ull_central_iso_setup(uint16_t cis_handle,
859859

860860
/* ACL connection of the new CIS */
861861
conn = ll_conn_get(cis->lll.acl_handle);
862+
LL_ASSERT(conn != NULL);
862863

863864
#if defined(CONFIG_BT_CTLR_JIT_SCHEDULING)
864865
uint16_t event_counter;
@@ -982,6 +983,7 @@ int ull_central_iso_cis_offset_get(uint16_t cis_handle,
982983
LL_ASSERT(cis);
983984

984985
conn = ll_conn_get(cis->lll.acl_handle);
986+
LL_ASSERT(conn != NULL);
985987

986988
/* `ull_conn_llcp()` (caller of this function) is called before `ull_ref_inc()` hence we do
987989
* not need to use `ull_conn_event_counter()`.
@@ -1060,10 +1062,12 @@ static void mfy_cig_offset_get(void *param)
10601062
(EVENT_TICKER_RES_MARGIN_US << 2U);
10611063
offset_min_us += cig->sync_delay - cis->sync_delay;
10621064

1065+
conn = ll_conn_get(cis->lll.acl_handle);
1066+
LL_ASSERT(conn != NULL);
1067+
10631068
/* Ensure the offset is not greater than the ACL interval, considering
10641069
* the minimum CIS offset requirement.
10651070
*/
1066-
conn = ll_conn_get(cis->lll.acl_handle);
10671071
conn_interval_us = (uint32_t)conn->lll.interval * CONN_INT_UNIT_US;
10681072
offset_limit_us = conn_interval_us + PDU_CIS_OFFSET_MIN_US;
10691073
while (offset_min_us >= offset_limit_us) {
@@ -1170,10 +1174,12 @@ static void mfy_cis_offset_get(void *param)
11701174
hal_ticker_remove_jitter(&ticks_to_expire, &remainder);
11711175
cig_remainder_us = remainder;
11721176

1177+
conn = ll_conn_get(cis->lll.acl_handle);
1178+
LL_ASSERT(conn != NULL);
1179+
11731180
/* Add a tick for negative remainder and return positive remainder
11741181
* value.
11751182
*/
1176-
conn = ll_conn_get(cis->lll.acl_handle);
11771183
remainder = conn->llcp.prep.remainder;
11781184
hal_ticker_add_jitter(&ticks_to_expire, &remainder);
11791185
acl_remainder_us = remainder;

subsys/bluetooth/controller/ll_sw/ull_conn.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -180,19 +180,19 @@ uint16_t ll_conn_handle_get(struct ll_conn *conn)
180180

181181
struct ll_conn *ll_conn_get(uint16_t handle)
182182
{
183+
if (handle >= CONFIG_BT_MAX_CONN) {
184+
return NULL;
185+
}
186+
183187
return mem_get(conn_pool, sizeof(struct ll_conn), handle);
184188
}
185189

186190
struct ll_conn *ll_connected_get(uint16_t handle)
187191
{
188192
struct ll_conn *conn;
189193

190-
if (handle >= CONFIG_BT_MAX_CONN) {
191-
return NULL;
192-
}
193-
194194
conn = ll_conn_get(handle);
195-
if (conn->lll.handle != handle) {
195+
if ((conn == NULL) || (conn->lll.handle != handle)) {
196196
return NULL;
197197
}
198198

@@ -429,6 +429,7 @@ uint8_t ll_terminate_ind_send(uint16_t handle, uint8_t reason)
429429
}
430430
return 0;
431431
}
432+
432433
#if defined(CONFIG_BT_CTLR_PERIPHERAL_ISO) || defined(CONFIG_BT_CTLR_CENTRAL_ISO)
433434
if (IS_CIS_HANDLE(handle)) {
434435
cis = ll_iso_stream_connected_get(handle);
@@ -445,6 +446,7 @@ uint8_t ll_terminate_ind_send(uint16_t handle, uint8_t reason)
445446

446447
} else if (cis->group->state == CIG_STATE_INITIATING) {
447448
conn = ll_connected_get(cis->lll.acl_handle);
449+
LL_ASSERT(conn != NULL);
448450

449451
/* CIS is not yet established - try to cancel procedure */
450452
if (ull_cp_cc_cancel(conn)) {
@@ -805,6 +807,9 @@ struct lll_conn *ull_conn_lll_get(uint16_t handle)
805807
struct ll_conn *conn;
806808

807809
conn = ll_conn_get(handle);
810+
if (conn == NULL) {
811+
return NULL;
812+
}
808813

809814
return &conn->lll;
810815
}
@@ -1553,22 +1558,32 @@ void ull_conn_tx_ack(uint16_t handle, memq_link_t *link, struct node_tx *tx)
15531558
if (handle != LLL_HANDLE_INVALID) {
15541559
struct ll_conn *conn = ll_conn_get(handle);
15551560

1561+
LL_ASSERT(conn != NULL);
1562+
15561563
ull_cp_tx_ack(conn, tx);
15571564
}
15581565

15591566
/* release ctrl mem if points to itself */
15601567
if (link->next == (void *)tx) {
1568+
struct ll_conn *conn;
1569+
1570+
/* Tx Node not re-used, ensure link->next is non-NULL */
15611571
LL_ASSERT(link->next);
15621572

1563-
struct ll_conn *conn = ll_connected_get(handle);
1573+
conn = ll_connected_get(handle);
1574+
LL_ASSERT(conn != NULL);
15641575

15651576
ull_cp_release_tx(conn, tx);
1577+
15661578
return;
1579+
15671580
} else if (!tx) {
15681581
/* Tx Node re-used to enqueue new ctrl PDU */
15691582
return;
15701583
}
1584+
15711585
LL_ASSERT(!link->next);
1586+
15721587
} else if (handle == LLL_HANDLE_INVALID) {
15731588
pdu_tx->ll_id = PDU_DATA_LLID_RESV;
15741589
} else {
@@ -1672,6 +1687,7 @@ static int init_reset(void)
16721687
for (uint16_t handle = 0U; handle < CONFIG_BT_MAX_CONN; handle++) {
16731688
struct ll_conn *conn;
16741689

1690+
/* handle in valid range, conn will be non-NULL */
16751691
conn = ll_conn_get(handle);
16761692
conn->lll.handle = LLL_HANDLE_INVALID;
16771693
}
@@ -1840,6 +1856,7 @@ static inline void disable(uint16_t handle)
18401856
int err;
18411857

18421858
conn = ll_conn_get(handle);
1859+
LL_ASSERT(conn != NULL);
18431860

18441861
err = ull_ticker_stop_with_mark(TICKER_ID_CONN_BASE + handle,
18451862
conn, &conn->lll);

subsys/bluetooth/controller/ll_sw/ull_conn_iso.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,8 @@ void ull_conn_iso_done(struct node_rx_event_done *done)
485485
if (!cis->event_expire) {
486486
struct ll_conn *conn = ll_conn_get(cis->lll.acl_handle);
487487

488+
LL_ASSERT(conn != NULL);
489+
488490
cis->event_expire = RADIO_CONN_EVENTS(
489491
conn->supervision_timeout * 10U * 1000U,
490492
cig->iso_interval * CONN_INT_UNIT_US);
@@ -526,8 +528,11 @@ void ull_conn_iso_done(struct node_rx_event_done *done)
526528
if (cis && (ticks_drift_plus || ticks_drift_minus)) {
527529
uint8_t ticker_id = TICKER_ID_CONN_ISO_BASE +
528530
ll_conn_iso_group_handle_get(cig);
529-
struct ll_conn *conn = ll_connected_get(cis->lll.acl_handle);
530531
uint32_t ticker_status;
532+
struct ll_conn *conn;
533+
534+
conn = ll_connected_get(cis->lll.acl_handle);
535+
LL_ASSERT(conn != NULL);
531536

532537
ticker_status = ticker_update(TICKER_INSTANCE_ID_CTLR,
533538
TICKER_USER_ID_ULL_HIGH,
@@ -1252,6 +1257,8 @@ static void cis_disabled_cb(void *param)
12521257
ll_iso_stream_released_cb_t cis_released_cb;
12531258

12541259
conn = ll_conn_get(cis->lll.acl_handle);
1260+
LL_ASSERT(conn != NULL);
1261+
12551262
cis_released_cb = cis->released_cb;
12561263
cis->released_cb = NULL;
12571264

@@ -1314,6 +1321,7 @@ static void cis_disabled_cb(void *param)
13141321
ll_rx_put_sched(node_terminate->hdr.link, node_terminate);
13151322
} else {
13161323
conn = ll_conn_get(cis->lll.acl_handle);
1324+
LL_ASSERT(conn != NULL);
13171325

13181326
/* CIS was not established - complete the procedure with error */
13191327
if (ull_cp_cc_awaiting_established(conn)) {

subsys/bluetooth/controller/ll_sw/ull_iso.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2023,7 +2023,10 @@ void ull_iso_resume_ticker_start(struct lll_event *resume_event,
20232023
struct ll_conn *conn;
20242024

20252025
cis = ll_conn_iso_stream_get(stream_handle);
2026+
20262027
conn = ll_conn_get(cis->lll.acl_handle);
2028+
LL_ASSERT(conn != NULL);
2029+
20272030
phy = conn->lll.phy_rx;
20282031
#endif /* CONFIG_BT_CTLR_CONN_ISO */
20292032
#if defined(CONFIG_BT_CTLR_SYNC_ISO)

subsys/bluetooth/controller/ll_sw/ull_peripheral_iso.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ static struct ll_conn *ll_cis_get_acl_awaiting_reply(uint16_t handle, uint8_t *e
6565
}
6666

6767
for (int h = 0; h < CONFIG_BT_MAX_CONN; h++) {
68+
/* Handle h in valid range, hence conn will be non-NULL */
6869
struct ll_conn *conn = ll_conn_get(h);
6970
uint16_t cis_handle = ull_cp_cc_ongoing_handle(conn);
7071

@@ -317,6 +318,7 @@ uint8_t ull_peripheral_iso_setup(struct pdu_data_llctrl_cis_ind *ind,
317318
}
318319

319320
conn = ll_conn_get(cis->lll.acl_handle);
321+
LL_ASSERT(conn != NULL);
320322

321323
cis_offset = sys_get_le24(ind->cis_offset);
322324

0 commit comments

Comments
 (0)