Skip to content

Commit 353e3d3

Browse files
BalaklakaHaavardRei
Balaklaka
authored and
HaavardRei
committed
[nrf fromtree] Bluetooth: Mesh: Fix out of bounds write
Fix of the subnet bridging table function to only compact the table if elements has been removed, and fixing the compact function to compact the table if several elemnts has been removed at the same time. Fixes zephyrproject-rtos#78794 Signed-off-by: Ingar Kulbrandstad <[email protected]> (cherry picked from commit b32eb0d) Signed-off-by: Håvard Reierstad <[email protected]>
1 parent 3eb9de0 commit 353e3d3

File tree

2 files changed

+147
-30
lines changed

2 files changed

+147
-30
lines changed

‎subsys/bluetooth/mesh/brg_cfg.c‎

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,18 @@ enum {
3333
};
3434
static ATOMIC_DEFINE(brg_cfg_flags, BRG_CFG_FLAGS_COUNT);
3535

36-
static void brg_tbl_compact(void)
36+
/* Compact the bridge table for all removed entries, input is first removed entry */
37+
static void brg_tbl_compact(int j)
3738
{
38-
int j = 0;
39-
40-
for (int k = 0; k < bt_mesh_brg_cfg_row_cnt; k++) {
39+
for (int k = j; k < bt_mesh_brg_cfg_row_cnt; k++) {
4140
if (brg_tbl[k].direction != 0) {
4241
brg_tbl[j] = brg_tbl[k];
4342
j++;
4443
}
4544
}
46-
memset(&brg_tbl[j], 0, sizeof(brg_tbl[j]));
47-
bt_mesh_brg_cfg_row_cnt--;
45+
46+
memset(&brg_tbl[j], 0, sizeof(brg_tbl[j]) * (bt_mesh_brg_cfg_row_cnt - j));
47+
bt_mesh_brg_cfg_row_cnt = j;
4848
}
4949

5050
/* Set function for initializing bridging enable state from value stored in settings. */
@@ -167,21 +167,30 @@ void bt_mesh_brg_cfg_pending_store(void)
167167
*/
168168
static void brg_tbl_netkey_removed_evt(struct bt_mesh_subnet *sub, enum bt_mesh_key_evt evt)
169169
{
170+
int first_removed = -1;
171+
170172
if (evt != BT_MESH_KEY_DELETED) {
171173
return;
172174
}
173175

174-
for (int i = 0; i < CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX; i++) {
175-
if (brg_tbl[i].direction &&
176-
(brg_tbl[i].net_idx1 == sub->net_idx || brg_tbl[i].net_idx2 == sub->net_idx)) {
177-
memset(&brg_tbl[i], 0, sizeof(brg_tbl[i]));
178-
brg_tbl_compact();
176+
for (int i = 0; i < bt_mesh_brg_cfg_row_cnt; i++) {
177+
if (brg_tbl[i].net_idx1 == sub->net_idx || brg_tbl[i].net_idx2 == sub->net_idx) {
178+
/* Setting direction to 0, entry will be cleared in brg_tbl_compact. */
179+
brg_tbl[i].direction = 0;
180+
if (first_removed == -1) {
181+
first_removed = i;
182+
}
179183
}
180184
}
181185

182-
if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
183-
atomic_set_bit(brg_cfg_flags, TABLE_UPDATED);
184-
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
186+
if (first_removed != -1) {
187+
/* Compact when all rows have been deleted. */
188+
brg_tbl_compact(first_removed);
189+
190+
if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
191+
atomic_set_bit(brg_cfg_flags, TABLE_UPDATED);
192+
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
193+
}
185194
}
186195
}
187196

@@ -263,7 +272,7 @@ int bt_mesh_brg_cfg_tbl_add(uint8_t direction, uint16_t net_idx1, uint16_t net_i
263272
}
264273

265274
/* Empty element, is the current table row counter */
266-
if (bt_mesh_brg_cfg_row_cnt == CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX) {
275+
if (bt_mesh_brg_cfg_row_cnt >= CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX) {
267276
*status = STATUS_INSUFF_RESOURCES;
268277
return 0;
269278
}
@@ -306,7 +315,7 @@ void bt_mesh_brg_cfg_tbl_foreach_subnet(uint16_t src, uint16_t dst, uint16_t net
306315
int bt_mesh_brg_cfg_tbl_remove(uint16_t net_idx1, uint16_t net_idx2, uint16_t addr1, uint16_t addr2,
307316
uint8_t *status)
308317
{
309-
bool store = false;
318+
int first_removed = -1;
310319

311320
/* Sanity checks */
312321
if ((!BT_MESH_ADDR_IS_UNICAST(addr1) && addr1 != BT_MESH_ADDR_UNASSIGNED) ||
@@ -334,26 +343,29 @@ int bt_mesh_brg_cfg_tbl_remove(uint16_t net_idx1, uint16_t net_idx2, uint16_t ad
334343

335344
for (int i = 0; i < bt_mesh_brg_cfg_row_cnt; i++) {
336345
/* Match according to remove behavior in Section 4.4.9.2.2 of MshPRT_v1.1 */
337-
if (brg_tbl[i].direction) {
338-
if (!(brg_tbl[i].net_idx1 == net_idx1 && brg_tbl[i].net_idx2 == net_idx2)) {
339-
continue;
340-
}
346+
if (!(brg_tbl[i].net_idx1 == net_idx1 && brg_tbl[i].net_idx2 == net_idx2)) {
347+
continue;
348+
}
341349

342-
if ((brg_tbl[i].addr1 == addr1 && brg_tbl[i].addr2 == addr2) ||
343-
(addr2 == BT_MESH_ADDR_UNASSIGNED && brg_tbl[i].addr1 == addr1) ||
344-
(addr1 == BT_MESH_ADDR_UNASSIGNED && brg_tbl[i].addr2 == addr2)) {
345-
memset(&brg_tbl[i], 0, sizeof(brg_tbl[i]));
346-
store = true;
350+
if ((brg_tbl[i].addr1 == addr1 && brg_tbl[i].addr2 == addr2) ||
351+
(addr2 == BT_MESH_ADDR_UNASSIGNED && brg_tbl[i].addr1 == addr1) ||
352+
(addr1 == BT_MESH_ADDR_UNASSIGNED && brg_tbl[i].addr2 == addr2)) {
353+
/* Setting direction to 0, entry will be cleared in brg_tbl_compact. */
354+
brg_tbl[i].direction = 0;
355+
if (first_removed == -1) {
356+
first_removed = i;
347357
}
348358
}
349359
}
350360

351-
/* Compact when all rows have been deleted. */
352-
brg_tbl_compact();
361+
if (first_removed != -1) {
362+
/* Compact when all rows have been deleted. */
363+
brg_tbl_compact(first_removed);
353364

354-
if (IS_ENABLED(CONFIG_BT_SETTINGS) && store) {
355-
atomic_set_bit(brg_cfg_flags, TABLE_UPDATED);
356-
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
365+
if (IS_ENABLED(CONFIG_BT_SETTINGS)) {
366+
atomic_set_bit(brg_cfg_flags, TABLE_UPDATED);
367+
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
368+
}
357369
}
358370

359371
*status = STATUS_SUCCESS;

‎tests/bluetooth/mesh/brg/src/main.c‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,111 @@ ZTEST(bt_mesh_brg_cfg, test_basic_functionality_storage)
218218
}
219219
}
220220

221+
static void check_bt_mesh_brg_cfg_tbl_multiple_delete(int expect_left)
222+
{
223+
uint8_t status;
224+
int err;
225+
int n;
226+
const struct bt_mesh_brg_cfg_row *brg_tbl;
227+
228+
n = bt_mesh_brg_cfg_tbl_get(&brg_tbl);
229+
zassert_equal(n, TEST_VECT_SZ - 1);
230+
231+
ztest_expect_value(bt_mesh_settings_store_schedule, flag, BT_MESH_SETTINGS_BRG_PENDING);
232+
err = bt_mesh_brg_cfg_tbl_remove(test_vector[1].net_idx1, test_vector[1].net_idx2,
233+
test_vector[1].addr1, BT_MESH_ADDR_UNASSIGNED, &status);
234+
zassert_equal(err, 0);
235+
236+
n = bt_mesh_brg_cfg_tbl_get(&brg_tbl);
237+
zassert_equal(n, expect_left);
238+
239+
for (int i = 0; i < n; i++) {
240+
zassert_true(brg_tbl[i].net_idx1 == test_vector[0].net_idx1);
241+
zassert_true(brg_tbl[i].net_idx2 == test_vector[0].net_idx2);
242+
zassert_true(brg_tbl[i].addr1 == test_vector[0].addr1);
243+
zassert_true(brg_tbl[i].addr2 == test_vector[i * 2].addr2);
244+
}
245+
}
246+
247+
ZTEST(bt_mesh_brg_cfg, test_removal_multiple_entries)
248+
{
249+
check_bt_mesh_brg_cfg_tbl_reset();
250+
251+
uint8_t status;
252+
int err;
253+
254+
/* Test removal of every second entry */
255+
for (int i = 0; i < TEST_VECT_SZ - 1; i++) {
256+
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
257+
BT_MESH_SETTINGS_BRG_PENDING);
258+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction, test_vector[i % 2].net_idx1,
259+
test_vector[i % 2].net_idx2, test_vector[i % 2].addr1,
260+
test_vector[i].addr2, &status);
261+
zassert_equal(err, 0);
262+
zassert_equal(status, STATUS_SUCCESS);
263+
}
264+
265+
check_bt_mesh_brg_cfg_tbl_multiple_delete((TEST_VECT_SZ - 1) / 2);
266+
check_bt_mesh_brg_cfg_tbl_reset();
267+
268+
/* Test removal of all entries, except first */
269+
for (int i = 0; i < TEST_VECT_SZ - 1; i++) {
270+
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
271+
BT_MESH_SETTINGS_BRG_PENDING);
272+
if (i == 0) {
273+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction,
274+
test_vector[i].net_idx1,
275+
test_vector[i].net_idx2, test_vector[i].addr1,
276+
test_vector[i].addr2, &status);
277+
} else {
278+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction,
279+
test_vector[1].net_idx1,
280+
test_vector[1].net_idx2, test_vector[1].addr1,
281+
test_vector[i].addr2, &status);
282+
}
283+
zassert_equal(err, 0);
284+
zassert_equal(status, STATUS_SUCCESS);
285+
}
286+
287+
check_bt_mesh_brg_cfg_tbl_multiple_delete(1);
288+
check_bt_mesh_brg_cfg_tbl_reset();
289+
290+
/* Test removal of all entries, except last */
291+
for (int i = TEST_VECT_SZ - 2; i >= 0; i--) {
292+
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
293+
BT_MESH_SETTINGS_BRG_PENDING);
294+
if (i == 0) {
295+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction,
296+
test_vector[i].net_idx1,
297+
test_vector[i].net_idx2, test_vector[i].addr1,
298+
test_vector[i].addr2, &status);
299+
} else {
300+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction,
301+
test_vector[1].net_idx1,
302+
test_vector[1].net_idx2, test_vector[1].addr1,
303+
test_vector[i].addr2, &status);
304+
}
305+
zassert_equal(err, 0);
306+
zassert_equal(status, STATUS_SUCCESS);
307+
}
308+
309+
check_bt_mesh_brg_cfg_tbl_multiple_delete(1);
310+
check_bt_mesh_brg_cfg_tbl_reset();
311+
312+
/* Test removal of all entries */
313+
for (int i = 0; i < TEST_VECT_SZ - 1; i++) {
314+
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
315+
BT_MESH_SETTINGS_BRG_PENDING);
316+
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction, test_vector[1].net_idx1,
317+
test_vector[1].net_idx2, test_vector[1].addr1,
318+
test_vector[i].addr2, &status);
319+
zassert_equal(err, 0);
320+
zassert_equal(status, STATUS_SUCCESS);
321+
}
322+
323+
check_bt_mesh_brg_cfg_tbl_multiple_delete(0);
324+
}
325+
221326
static void pending_store_enable_create_expectations(bool *enable_val)
222327
{
223328
if (*enable_val) {

0 commit comments

Comments
 (0)