bluetooth: mesh: brg_cfg_srv: ignore message with invalid parameters
When a message with invalid parameters is received, we must ignore it.
In this commit we check invalid parameters first.
Signed-off-by: Pavel Vasilyev <pavel.vasilyev@nordicsemi.no>
diff --git a/subsys/bluetooth/mesh/brg_cfg.c b/subsys/bluetooth/mesh/brg_cfg.c
index c6974b9..d13e1a5 100644
--- a/subsys/bluetooth/mesh/brg_cfg.c
+++ b/subsys/bluetooth/mesh/brg_cfg.c
@@ -14,6 +14,7 @@
#include "net.h"
#include "settings.h"
#include "brg_cfg.h"
+#include "foundation.h"
#define LOG_LEVEL CONFIG_BT_MESH_BRG_LOG_LEVEL
#include <zephyr/logging/log.h>
@@ -219,26 +220,38 @@
return bt_mesh_brg_cfg_row_cnt;
}
+static bool netkey_check(uint16_t net_idx1, uint16_t net_idx2)
+{
+ return bt_mesh_subnet_get(net_idx1) && bt_mesh_subnet_get(net_idx2);
+}
+
int bt_mesh_brg_cfg_tbl_add(enum bt_mesh_brg_cfg_dir direction, uint16_t net_idx1,
- uint16_t net_idx2, uint16_t addr1, uint16_t addr2)
+ uint16_t net_idx2, uint16_t addr1, uint16_t addr2, uint8_t *status)
{
/* Sanity checks */
- if (!BT_MESH_ADDR_IS_UNICAST(addr1) || net_idx1 == net_idx2 || addr1 == addr2 ||
- net_idx1 > BT_MESH_BRG_CFG_KEY_INDEX_MAX || net_idx2 > BT_MESH_BRG_CFG_KEY_INDEX_MAX) {
+ if (!BT_MESH_ADDR_IS_UNICAST(addr1) || net_idx1 == net_idx2 ||
+ addr1 == addr2 || net_idx1 > BT_MESH_BRG_CFG_KEY_INDEX_MAX ||
+ net_idx2 > BT_MESH_BRG_CFG_KEY_INDEX_MAX) {
return -EINVAL;
}
- if (direction != BT_MESH_BRG_CFG_DIR_ONEWAY && direction != BT_MESH_BRG_CFG_DIR_TWOWAY) {
+ if (direction != BT_MESH_BRG_CFG_DIR_ONEWAY &&
+ direction != BT_MESH_BRG_CFG_DIR_TWOWAY) {
return -EINVAL;
}
if ((direction == BT_MESH_BRG_CFG_DIR_ONEWAY &&
- (addr2 == BT_MESH_ADDR_UNASSIGNED || addr2 == BT_MESH_ADDR_ALL_NODES)) ||
+ (addr2 == BT_MESH_ADDR_UNASSIGNED || addr2 == BT_MESH_ADDR_ALL_NODES)) ||
(direction == BT_MESH_BRG_CFG_DIR_TWOWAY &&
- !BT_MESH_ADDR_IS_UNICAST(addr2))) {
+ !BT_MESH_ADDR_IS_UNICAST(addr2))) {
return -EINVAL;
}
+ if (!netkey_check(net_idx1, net_idx2)) {
+ *status = STATUS_INVALID_NETKEY;
+ return 0;
+ }
+
/* Check if entry already exists, if yes, then, update the direction field and it is a
* success.
* "If a Bridging Table state entry corresponding to the received message exists, the
@@ -256,7 +269,8 @@
/* Empty element, is the current table row counter */
if (bt_mesh_brg_cfg_row_cnt == CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX) {
- return -ENOMEM;
+ *status = STATUS_INSUFF_RESOURCES;
+ return 0;
}
/* Update the row */
@@ -273,6 +287,7 @@
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
}
+ *status = STATUS_SUCCESS;
return 0;
}
@@ -294,12 +309,13 @@
}
int bt_mesh_brg_cfg_tbl_remove(uint16_t net_idx1, uint16_t net_idx2, uint16_t addr1,
- uint16_t addr2)
+ uint16_t addr2, uint8_t *status)
{
bool store = false;
/* Sanity checks */
if ((!BT_MESH_ADDR_IS_UNICAST(addr1) && addr1 != BT_MESH_ADDR_UNASSIGNED) ||
+ (BT_MESH_ADDR_IS_UNICAST(addr1) && addr1 == addr2) ||
addr2 == BT_MESH_ADDR_ALL_NODES) {
return -EINVAL;
}
@@ -309,11 +325,16 @@
return -EINVAL;
}
+ if (!netkey_check(net_idx1, net_idx2)) {
+ *status = STATUS_INVALID_NETKEY;
+ return 0;
+ }
/* Iterate over items and set matching row to 0, if nothing exist, or nothing matches, then
* it is success (similar to add)
*/
if (bt_mesh_brg_cfg_row_cnt == 0) {
+ *status = STATUS_SUCCESS;
return 0;
}
@@ -341,5 +362,6 @@
bt_mesh_settings_store_schedule(BT_MESH_SETTINGS_BRG_PENDING);
}
+ *status = STATUS_SUCCESS;
return 0;
}
diff --git a/subsys/bluetooth/mesh/brg_cfg.h b/subsys/bluetooth/mesh/brg_cfg.h
index 033572e..95455c4 100644
--- a/subsys/bluetooth/mesh/brg_cfg.h
+++ b/subsys/bluetooth/mesh/brg_cfg.h
@@ -51,10 +51,10 @@
int bt_mesh_brg_cfg_tbl_get(const struct bt_mesh_brg_cfg_row **rows);
int bt_mesh_brg_cfg_tbl_add(enum bt_mesh_brg_cfg_dir direction, uint16_t net_idx1,
- uint16_t net_idx2, uint16_t addr1, uint16_t addr2);
+ uint16_t net_idx2, uint16_t addr1, uint16_t addr2, uint8_t *status);
int bt_mesh_brg_cfg_tbl_remove(uint16_t net_idx1, uint16_t net_idx2, uint16_t addr1,
- uint16_t addr2);
+ uint16_t addr2, uint8_t *status);
typedef void (*bt_mesh_brg_cfg_cb_t)(uint16_t new_netidx, void *user_data);
diff --git a/subsys/bluetooth/mesh/brg_cfg_srv.c b/subsys/bluetooth/mesh/brg_cfg_srv.c
index adfe5de..581dd98 100644
--- a/subsys/bluetooth/mesh/brg_cfg_srv.c
+++ b/subsys/bluetooth/mesh/brg_cfg_srv.c
@@ -78,28 +78,19 @@
{
struct bt_mesh_bridging_table_entry entry;
uint8_t status = STATUS_SUCCESS;
+ int err;
entry.directions = net_buf_simple_pull_u8(buf);
key_idx_unpack_pair(buf, &entry.net_idx1, &entry.net_idx2);
entry.addr1 = net_buf_simple_pull_le16(buf);
entry.addr2 = net_buf_simple_pull_le16(buf);
- if (!netkey_check(entry.net_idx1, entry.net_idx2)) {
- status = STATUS_INVALID_NETKEY;
- goto add_respond;
- }
-
- int err = bt_mesh_brg_cfg_tbl_add(entry.directions, entry.net_idx1, entry.net_idx2,
- entry.addr1, entry.addr2);
- if (err == -ENOMEM) {
- status = STATUS_INSUFF_RESOURCES;
- } else if (err) {
- /* Per spec, do not respond if parameters values are invalid. */
+ err = bt_mesh_brg_cfg_tbl_add(entry.directions, entry.net_idx1, entry.net_idx2, entry.addr1,
+ entry.addr2, &status);
+ if (err) {
return err;
}
-add_respond:
-
bridging_table_status_send(model, ctx, status, &entry);
return 0;
@@ -110,27 +101,19 @@
{
struct bt_mesh_bridging_table_entry entry;
uint8_t status = STATUS_SUCCESS;
+ int err;
entry.directions = 0;
key_idx_unpack_pair(buf, &entry.net_idx1, &entry.net_idx2);
entry.addr1 = net_buf_simple_pull_le16(buf);
entry.addr2 = net_buf_simple_pull_le16(buf);
- if (!netkey_check(entry.net_idx1, entry.net_idx2)) {
- status = STATUS_INVALID_NETKEY;
- goto rmv_respond;
- }
-
- int err = bt_mesh_brg_cfg_tbl_remove(entry.net_idx1, entry.net_idx2, entry.addr1,
- entry.addr2);
-
- /* Per spec, do not respond if parameters values are invalid. */
+ err = bt_mesh_brg_cfg_tbl_remove(entry.net_idx1, entry.net_idx2, entry.addr1, entry.addr2,
+ &status);
if (err) {
return err;
}
-rmv_respond:
-
bridging_table_status_send(model, ctx, status, &entry);
return 0;
diff --git a/tests/bluetooth/mesh/brg/src/main.c b/tests/bluetooth/mesh/brg/src/main.c
index 68103c9..15d964f 100644
--- a/tests/bluetooth/mesh/brg/src/main.c
+++ b/tests/bluetooth/mesh/brg/src/main.c
@@ -11,6 +11,7 @@
#include "settings.h"
#include "brg_cfg.h"
+#include "foundation.h"
#define TEST_VECT_SZ (CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX + 1)
@@ -36,7 +37,6 @@
test_vector[i].net_idx2 = (i/8) + 16;
test_vector[i].addr2 = ADDR2_BASE + i;
}
-
}
/**** Mocked functions ****/
@@ -60,10 +60,17 @@
return 0;
}
+struct bt_mesh_subnet *bt_mesh_subnet_get(uint16_t net_idx)
+{
+ /* Return anything non-zero. */
+ return (struct bt_mesh_subnet *) 1;
+}
+
/**** Mocked functions - end ****/
static void check_fill_all_bt_entries(void)
{
+ uint8_t status;
int err;
for (int i = 0; i < TEST_VECT_SZ; i++) {
@@ -74,18 +81,23 @@
}
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction, test_vector[i].net_idx1,
- test_vector[i].net_idx2, test_vector[i].addr1, test_vector[i].addr2);
+ test_vector[i].net_idx2, test_vector[i].addr1, test_vector[i].addr2,
+ &status);
+
+ zassert_equal(err, 0);
if (i != CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX) {
- zassert_equal(err, 0);
+ zassert_equal(status, STATUS_SUCCESS);
} else {
- zassert_equal(err, -ENOMEM);
+ zassert_equal(status, STATUS_INSUFF_RESOURCES);
}
}
}
static void check_delete_all_bt_entries(void)
{
+ uint8_t status;
+
for (int i = 0; i < TEST_VECT_SZ; i++) {
if (i < CONFIG_BT_MESH_BRG_TABLE_ITEMS_MAX) {
@@ -95,9 +107,10 @@
int err = bt_mesh_brg_cfg_tbl_remove(test_vector[i].net_idx1,
test_vector[i].net_idx2, test_vector[i].addr1,
- test_vector[i].addr2);
+ test_vector[i].addr2, &status);
zassert_equal(err, 0);
+ zassert_equal(status, STATUS_SUCCESS);
}
}
@@ -132,6 +145,7 @@
check_bt_mesh_brg_cfg_tbl_reset();
check_fill_all_bt_entries();
+ uint8_t status;
int err;
uint16_t net_idx1 = test_vector[TEST_VECT_SZ - 1].net_idx1;
uint16_t net_idx2 = test_vector[TEST_VECT_SZ - 1].net_idx2;
@@ -140,27 +154,27 @@
/* Try removing entries with invalid params */
uint16_t addr2 = BT_MESH_ADDR_ALL_NODES;
- err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2);
+ err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2, &status);
zassert_equal(err, -EINVAL);
addr2 = BT_MESH_ADDR_UNASSIGNED;
addr1 = BT_MESH_ADDR_RELAYS;
- err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2);
+ err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2, &status);
zassert_equal(err, -EINVAL);
addr1 = BT_MESH_ADDR_UNASSIGNED;
net_idx1 = 4096;
- err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2);
+ err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2, &status);
zassert_equal(err, -EINVAL);
net_idx1 = test_vector[TEST_VECT_SZ - 1].net_idx1;
net_idx2 = 4096;
- err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2);
+ err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2, &status);
zassert_equal(err, -EINVAL);
/* Test remove entries matching netkey1, and netkey2 */
net_idx2 = test_vector[TEST_VECT_SZ - 1].net_idx2;
- err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2);
+ err = bt_mesh_brg_cfg_tbl_remove(net_idx1, net_idx2, addr1, addr2, &status);
zassert_equal(err, 0);
const struct bt_mesh_brg_cfg_row *brg_tbl;
@@ -255,6 +269,7 @@
/* Test if pending store works correctly by adding one entry to the table. */
ZTEST(bt_mesh_brg_cfg, test_brg_tbl_pending_store)
{
+ uint8_t status;
int n, err;
struct bt_mesh_brg_cfg_row test_vec = {
.direction = BT_MESH_BRG_CFG_DIR_ONEWAY,
@@ -268,8 +283,9 @@
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
BT_MESH_SETTINGS_BRG_PENDING);
err = bt_mesh_brg_cfg_tbl_add(test_vec.direction, test_vec.net_idx1,
- test_vec.net_idx2, test_vec.addr1, test_vec.addr2);
+ test_vec.net_idx2, test_vec.addr1, test_vec.addr2, &status);
zassert_equal(err, 0);
+ zassert_equal(status, STATUS_SUCCESS);
const struct bt_mesh_brg_cfg_row *tbl;
@@ -285,6 +301,7 @@
/* Test if invalid entries are not added to the table. */
ZTEST(bt_mesh_brg_cfg, test_tbl_add_invalid_ip)
{
+ uint8_t status;
int err;
/* Create test vector array of test_brg_cfg_row iteams with invalid values.
* Each vector has only one invalid field value, rest all are valid values.
@@ -337,7 +354,8 @@
for (int i = 0; i < ARRAY_SIZE(inv_test_vector); i++) {
err = bt_mesh_brg_cfg_tbl_add(inv_test_vector[i].direction,
inv_test_vector[i].net_idx1, inv_test_vector[i].net_idx2,
- inv_test_vector[i].addr1, inv_test_vector[i].addr2);
+ inv_test_vector[i].addr1, inv_test_vector[i].addr2,
+ &status);
zassert_equal(err, -EINVAL, "Test vector index: %zu", i);
}
}
@@ -362,27 +380,32 @@
static void check_fill_all_bt_entries_reversed(void)
{
+ uint8_t status;
int err;
for (int i = TEST_VECT_SZ - 2; i >= 0 ; i--) {
ztest_expect_value(bt_mesh_settings_store_schedule, flag,
BT_MESH_SETTINGS_BRG_PENDING);
err = bt_mesh_brg_cfg_tbl_add(test_vector[i].direction, test_vector[i].net_idx1,
- test_vector[i].net_idx2, test_vector[i].addr1, test_vector[i].addr2);
+ test_vector[i].net_idx2, test_vector[i].addr1, test_vector[i].addr2,
+ &status);
zassert_equal(err, 0);
}
int last = TEST_VECT_SZ - 1;
err = bt_mesh_brg_cfg_tbl_add(test_vector[last].direction, test_vector[last].net_idx1,
- test_vector[last].net_idx2, test_vector[last].addr1, test_vector[last].addr2);
- zassert_equal(err, -ENOMEM);
+ test_vector[last].net_idx2, test_vector[last].addr1,
+ test_vector[last].addr2, &status);
+ zassert_equal(err, 0);
+ zassert_equal(status, STATUS_INSUFF_RESOURCES);
}
static struct test_brg_cfg_row test_vector_copy[TEST_VECT_SZ - 1];
static void check_fill_all_bt_entries_randomly(void)
{
+ uint8_t status;
int err;
int copy_cnt = ARRAY_SIZE(test_vector_copy);
@@ -401,15 +424,18 @@
BT_MESH_SETTINGS_BRG_PENDING);
err = bt_mesh_brg_cfg_tbl_add(test_vector_copy[i].direction,
test_vector_copy[i].net_idx1, test_vector_copy[i].net_idx2,
- test_vector_copy[i].addr1, test_vector_copy[i].addr2);
+ test_vector_copy[i].addr1, test_vector_copy[i].addr2, &status);
zassert_equal(err, 0);
+ zassert_equal(status, STATUS_SUCCESS);
}
int last = TEST_VECT_SZ - 1;
err = bt_mesh_brg_cfg_tbl_add(test_vector[last].direction, test_vector[last].net_idx1,
- test_vector[last].net_idx2, test_vector[last].addr1, test_vector[last].addr2);
- zassert_equal(err, -ENOMEM);
+ test_vector[last].net_idx2, test_vector[last].addr1, test_vector[last].addr2,
+ &status);
+ zassert_equal(err, 0);
+ zassert_equal(status, STATUS_INSUFF_RESOURCES);
}
static void subnet_relay_cb_check(uint16_t new_net_idx, void *user_data)
