Skip to content

Commit 933a987

Browse files
MariuszSkamracarlescufi
MariuszSkamra
authored and
carlescufi
committed
Bluetooth: audio: Fix possible memory violation
This fixes possible memory violation by using the index outside of an instance array by tbs_client_common_call_control. This basically fixes the get_inst_by_index function to return NULL if the index is invalid. The function calls have been guarded to catch the returned NULL. Signed-off-by: Mariusz Skamra <[email protected]>
1 parent 25fe4ef commit 933a987

File tree

1 file changed

+86
-71
lines changed

1 file changed

+86
-71
lines changed

‎subsys/bluetooth/audio/tbs_client.c‎

Lines changed: 86 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -50,35 +50,30 @@ static const struct bt_uuid *gtbs_uuid = BT_UUID_GTBS;
5050

5151
static void discover_next_instance(struct bt_conn *conn, uint8_t index);
5252

53-
static bool valid_inst_index(struct bt_conn *conn, uint8_t idx)
53+
static struct bt_tbs_instance *tbs_inst_by_index(struct bt_conn *conn, uint8_t index)
5454
{
55-
uint8_t conn_index;
55+
struct bt_tbs_server_inst *server;
5656

5757
__ASSERT(conn, "NULL conn");
5858

59-
conn_index = bt_conn_index(conn);
60-
61-
if (IS_ENABLED(CONFIG_BT_TBS_CLIENT_GTBS) && idx == BT_TBS_GTBS_INDEX) {
62-
return true;
63-
} else {
64-
return idx < srv_insts[conn_index].inst_cnt;
65-
}
66-
}
67-
68-
static struct bt_tbs_instance *get_inst_by_index(struct bt_conn *conn,
69-
uint8_t idx)
70-
{
71-
uint8_t conn_index;
59+
server = &srv_insts[bt_conn_index(conn)];
7260

73-
__ASSERT(conn, "NULL conn");
61+
if (IS_ENABLED(CONFIG_BT_TBS_CLIENT_GTBS)) {
62+
/* GTBS can be accessed by BT_TBS_GTBS_INDEX only */
63+
if (index == GTBS_INDEX) {
64+
return NULL;
65+
}
7466

75-
conn_index = bt_conn_index(conn);
67+
if (index == BT_TBS_GTBS_INDEX) {
68+
return &server->tbs_insts[GTBS_INDEX];
69+
}
70+
}
7671

77-
if (IS_ENABLED(CONFIG_BT_TBS_CLIENT_GTBS) && idx == BT_TBS_GTBS_INDEX) {
78-
return &srv_insts[conn_index].tbs_insts[GTBS_INDEX];
79-
} else {
80-
return &srv_insts[conn_index].tbs_insts[idx];
72+
if (index < ARRAY_SIZE(server->tbs_insts)) {
73+
return &server->tbs_insts[index];
8174
}
75+
76+
return NULL;
8277
}
8378

8479
#if defined(CONFIG_BT_TBS_CLIENT_ORIGINATE_CALL)
@@ -592,9 +587,10 @@ static int tbs_client_common_call_control(struct bt_conn *conn,
592587
uint8_t call_index,
593588
uint8_t opcode)
594589
{
595-
const struct bt_tbs_instance *inst = get_inst_by_index(conn, inst_index);
590+
struct bt_tbs_instance *inst;
596591
struct bt_tbs_call_cp_acc common;
597592

593+
inst = tbs_inst_by_index(conn, inst_index);
598594
if (inst == NULL) {
599595
return -EINVAL;
600596
}
@@ -1718,14 +1714,15 @@ int bt_tbs_client_originate_call(struct bt_conn *conn, uint8_t inst_index,
17181714

17191715
if (conn == NULL) {
17201716
return -ENOTCONN;
1721-
} else if (!valid_inst_index(conn, inst_index)) {
1722-
return -EINVAL;
17231717
} else if (!bt_tbs_valid_uri(uri)) {
17241718
BT_DBG("Invalid URI: %s", uri);
17251719
return -EINVAL;
17261720
}
17271721

1728-
inst = get_inst_by_index(conn, inst_index);
1722+
inst = tbs_inst_by_index(conn, inst_index);
1723+
if (inst == NULL) {
1724+
return -EINVAL;
1725+
}
17291726

17301727
/* Check if there are free spots */
17311728
if (!free_call_spot(inst)) {
@@ -1768,10 +1765,12 @@ int bt_tbs_client_join_calls(struct bt_conn *conn, uint8_t inst_index,
17681765
uint8_t write_buf[CONFIG_BT_L2CAP_TX_MTU];
17691766
const size_t max_call_cnt = sizeof(write_buf) - sizeof(join->opcode);
17701767

1771-
inst = get_inst_by_index(conn, inst_index);
1768+
inst = tbs_inst_by_index(conn, inst_index);
17721769
if (inst == NULL) {
17731770
return -EINVAL;
1774-
} else if (inst->call_cp_sub_params.value_handle == 0) {
1771+
}
1772+
1773+
if (inst->call_cp_sub_params.value_handle == 0) {
17751774
BT_DBG("Handle not set");
17761775
return -EINVAL;
17771776
}
@@ -1807,11 +1806,13 @@ int bt_tbs_client_set_signal_strength_interval(struct bt_conn *conn,
18071806

18081807
if (conn == NULL) {
18091808
return -ENOTCONN;
1810-
} else if (!valid_inst_index(conn, inst_index)) {
1809+
}
1810+
1811+
inst = tbs_inst_by_index(conn, inst_index);
1812+
if (inst == NULL) {
18111813
return -EINVAL;
18121814
}
18131815

1814-
inst = get_inst_by_index(conn, inst_index);
18151816
/* Populate Outgoing Remote URI */
18161817
if (inst->signal_interval_handle == 0) {
18171818
BT_DBG("Handle not set");
@@ -1834,11 +1835,12 @@ int bt_tbs_client_read_bearer_provider_name(struct bt_conn *conn,
18341835

18351836
if (conn == NULL) {
18361837
return -ENOTCONN;
1837-
} else if (!valid_inst_index(conn, inst_index)) {
1838-
return -EINVAL;
18391838
}
18401839

1841-
inst = get_inst_by_index(conn, inst_index);
1840+
inst = tbs_inst_by_index(conn, inst_index);
1841+
if (inst == NULL) {
1842+
return -EINVAL;
1843+
}
18421844

18431845
if (inst->name_sub_params.value_handle == 0) {
18441846
BT_DBG("Handle not set");
@@ -1867,11 +1869,12 @@ int bt_tbs_client_read_bearer_uci(struct bt_conn *conn, uint8_t inst_index)
18671869

18681870
if (conn == NULL) {
18691871
return -ENOTCONN;
1870-
} else if (!valid_inst_index(conn, inst_index)) {
1871-
return -EINVAL;
18721872
}
18731873

1874-
inst = get_inst_by_index(conn, inst_index);
1874+
inst = tbs_inst_by_index(conn, inst_index);
1875+
if (inst == NULL) {
1876+
return -EINVAL;
1877+
}
18751878

18761879
if (inst->bearer_uci_handle == 0) {
18771880
BT_DBG("Handle not set");
@@ -1900,11 +1903,12 @@ int bt_tbs_client_read_technology(struct bt_conn *conn, uint8_t inst_index)
19001903

19011904
if (conn == NULL) {
19021905
return -ENOTCONN;
1903-
} else if (!valid_inst_index(conn, inst_index)) {
1904-
return -EINVAL;
19051906
}
19061907

1907-
inst = get_inst_by_index(conn, inst_index);
1908+
inst = tbs_inst_by_index(conn, inst_index);
1909+
if (inst == NULL) {
1910+
return -EINVAL;
1911+
}
19081912

19091913
if (inst->technology_sub_params.value_handle == 0) {
19101914
BT_DBG("Handle not set");
@@ -1933,11 +1937,12 @@ int bt_tbs_client_read_uri_list(struct bt_conn *conn, uint8_t inst_index)
19331937

19341938
if (conn == NULL) {
19351939
return -ENOTCONN;
1936-
} else if (!valid_inst_index(conn, inst_index)) {
1937-
return -EINVAL;
19381940
}
19391941

1940-
inst = get_inst_by_index(conn, inst_index);
1942+
inst = tbs_inst_by_index(conn, inst_index);
1943+
if (inst == NULL) {
1944+
return -EINVAL;
1945+
}
19411946

19421947
if (inst->uri_list_handle == 0) {
19431948
BT_DBG("Handle not set");
@@ -1966,11 +1971,12 @@ int bt_tbs_client_read_signal_strength(struct bt_conn *conn, uint8_t inst_index)
19661971

19671972
if (conn == NULL) {
19681973
return -ENOTCONN;
1969-
} else if (!valid_inst_index(conn, inst_index)) {
1970-
return -EINVAL;
19711974
}
19721975

1973-
inst = get_inst_by_index(conn, inst_index);
1976+
inst = tbs_inst_by_index(conn, inst_index);
1977+
if (inst == NULL) {
1978+
return -EINVAL;
1979+
}
19741980

19751981
if (inst->signal_strength_sub_params.value_handle == 0) {
19761982
BT_DBG("Handle not set");
@@ -1999,11 +2005,12 @@ int bt_tbs_client_read_signal_interval(struct bt_conn *conn, uint8_t inst_index)
19992005

20002006
if (conn == NULL) {
20012007
return -ENOTCONN;
2002-
} else if (!valid_inst_index(conn, inst_index)) {
2003-
return -EINVAL;
20042008
}
20052009

2006-
inst = get_inst_by_index(conn, inst_index);
2010+
inst = tbs_inst_by_index(conn, inst_index);
2011+
if (inst == NULL) {
2012+
return -EINVAL;
2013+
}
20072014

20082015
if (inst->signal_interval_handle == 0) {
20092016
BT_DBG("Handle not set");
@@ -2032,11 +2039,12 @@ int bt_tbs_client_read_current_calls(struct bt_conn *conn, uint8_t inst_index)
20322039

20332040
if (conn == NULL) {
20342041
return -ENOTCONN;
2035-
} else if (!valid_inst_index(conn, inst_index)) {
2036-
return -EINVAL;
20372042
}
20382043

2039-
inst = get_inst_by_index(conn, inst_index);
2044+
inst = tbs_inst_by_index(conn, inst_index);
2045+
if (inst == NULL) {
2046+
return -EINVAL;
2047+
}
20402048

20412049
if (inst->current_calls_sub_params.value_handle == 0) {
20422050
BT_DBG("Handle not set");
@@ -2065,11 +2073,12 @@ int bt_tbs_client_read_ccid(struct bt_conn *conn, uint8_t inst_index)
20652073

20662074
if (conn == NULL) {
20672075
return -ENOTCONN;
2068-
} else if (!valid_inst_index(conn, inst_index)) {
2069-
return -EINVAL;
20702076
}
20712077

2072-
inst = get_inst_by_index(conn, inst_index);
2078+
inst = tbs_inst_by_index(conn, inst_index);
2079+
if (inst == NULL) {
2080+
return -EINVAL;
2081+
}
20732082

20742083
if (inst->ccid_handle == 0) {
20752084
BT_DBG("Handle not set");
@@ -2098,11 +2107,12 @@ int bt_tbs_client_read_call_uri(struct bt_conn *conn, uint8_t inst_index)
20982107

20992108
if (conn == NULL) {
21002109
return -ENOTCONN;
2101-
} else if (!valid_inst_index(conn, inst_index)) {
2102-
return -EINVAL;
21032110
}
21042111

2105-
inst = get_inst_by_index(conn, inst_index);
2112+
inst = tbs_inst_by_index(conn, inst_index);
2113+
if (inst == NULL) {
2114+
return -EINVAL;
2115+
}
21062116

21072117
if (inst->in_target_uri_sub_params.value_handle == 0) {
21082118
BT_DBG("Handle not set");
@@ -2131,11 +2141,12 @@ int bt_tbs_client_read_status_flags(struct bt_conn *conn, uint8_t inst_index)
21312141

21322142
if (conn == NULL) {
21332143
return -ENOTCONN;
2134-
} else if (!valid_inst_index(conn, inst_index)) {
2135-
return -EINVAL;
21362144
}
21372145

2138-
inst = get_inst_by_index(conn, inst_index);
2146+
inst = tbs_inst_by_index(conn, inst_index);
2147+
if (inst == NULL) {
2148+
return -EINVAL;
2149+
}
21392150

21402151
if (inst->status_flags_sub_params.value_handle == 0) {
21412152
BT_DBG("Handle not set");
@@ -2163,11 +2174,12 @@ int bt_tbs_client_read_call_state(struct bt_conn *conn, uint8_t inst_index)
21632174

21642175
if (conn == NULL) {
21652176
return -ENOTCONN;
2166-
} else if (!valid_inst_index(conn, inst_index)) {
2167-
return -EINVAL;
21682177
}
21692178

2170-
inst = get_inst_by_index(conn, inst_index);
2179+
inst = tbs_inst_by_index(conn, inst_index);
2180+
if (inst == NULL) {
2181+
return -EINVAL;
2182+
}
21712183

21722184
if (inst->call_state_sub_params.value_handle == 0) {
21732185
BT_DBG("Handle not set");
@@ -2196,11 +2208,12 @@ int bt_tbs_client_read_optional_opcodes(struct bt_conn *conn,
21962208

21972209
if (conn == NULL) {
21982210
return -ENOTCONN;
2199-
} else if (!valid_inst_index(conn, inst_index)) {
2200-
return -EINVAL;
22012211
}
22022212

2203-
inst = get_inst_by_index(conn, inst_index);
2213+
inst = tbs_inst_by_index(conn, inst_index);
2214+
if (inst == NULL) {
2215+
return -EINVAL;
2216+
}
22042217

22052218
if (inst->optional_opcodes_handle == 0) {
22062219
BT_DBG("Handle not set");
@@ -2229,11 +2242,12 @@ int bt_tbs_client_read_remote_uri(struct bt_conn *conn, uint8_t inst_index)
22292242

22302243
if (conn == NULL) {
22312244
return -ENOTCONN;
2232-
} else if (!valid_inst_index(conn, inst_index)) {
2233-
return -EINVAL;
22342245
}
22352246

2236-
inst = get_inst_by_index(conn, inst_index);
2247+
inst = tbs_inst_by_index(conn, inst_index);
2248+
if (inst == NULL) {
2249+
return -EINVAL;
2250+
}
22372251

22382252
if (inst->incoming_call_sub_params.value_handle == 0) {
22392253
BT_DBG("Handle not set");
@@ -2262,11 +2276,12 @@ int bt_tbs_client_read_friendly_name(struct bt_conn *conn, uint8_t inst_index)
22622276

22632277
if (conn == NULL) {
22642278
return -ENOTCONN;
2265-
} else if (!valid_inst_index(conn, inst_index)) {
2266-
return -EINVAL;
22672279
}
22682280

2269-
inst = get_inst_by_index(conn, inst_index);
2281+
inst = tbs_inst_by_index(conn, inst_index);
2282+
if (inst == NULL) {
2283+
return -EINVAL;
2284+
}
22702285

22712286
if (inst->friendly_name_sub_params.value_handle == 0) {
22722287
BT_DBG("Handle not set");

0 commit comments

Comments
 (0)