2023-03-16 01:16:37

by Brian Gix

[permalink] [raw]
Subject: [PATCH BlueZ v2 1/3] mesh: Filter originated Provisioning Data packets

The mesh daemon can process incoming mesh packets on more than one
controller, but if a Provisioning data packet that originated from the
local daemon is received by a different controller, it must be filtered
and disregarded, or it will break the provisioning protocol.
---
mesh/mesh-io-mgmt.c | 58 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/mesh/mesh-io-mgmt.c b/mesh/mesh-io-mgmt.c
index 9ae1af05a..65994f058 100644
--- a/mesh/mesh-io-mgmt.c
+++ b/mesh/mesh-io-mgmt.c
@@ -82,6 +82,8 @@ struct dup_filter {
uint8_t addr[6];
} __packed;

+static const uint8_t zero_addr[] = {0, 0, 0, 0, 0, 0};
+
static struct mesh_io_private *pvt;

static uint32_t get_instant(void)
@@ -110,6 +112,14 @@ static bool find_by_addr(const void *a, const void *b)
return !memcmp(filter->addr, b, 6);
}

+static bool find_by_adv(const void *a, const void *b)
+{
+ const struct dup_filter *filter = a;
+ uint64_t data = l_get_be64(b);
+
+ return !memcmp(filter->addr, zero_addr, 6) && filter->data == data;
+}
+
static void filter_timeout(struct l_timeout *timeout, void *user_data)
{
struct dup_filter *filter;
@@ -146,7 +156,22 @@ static bool filter_dups(const uint8_t *addr, const uint8_t *adv,
uint32_t instant_delta;
uint64_t data = l_get_be64(adv);

- filter = l_queue_remove_if(pvt->dup_filters, find_by_addr, addr);
+ if (!addr)
+ addr = zero_addr;
+
+ if (adv[1] == MESH_AD_TYPE_PROVISION) {
+ filter = l_queue_find(pvt->dup_filters, find_by_adv, adv);
+
+ if (!filter && addr != zero_addr)
+ return false;
+
+ l_queue_remove(pvt->dup_filters, filter);
+
+ } else {
+ filter = l_queue_remove_if(pvt->dup_filters, find_by_addr,
+ addr);
+ }
+
if (!filter) {
filter = l_new(struct dup_filter, 1);
memcpy(filter->addr, addr, 6);
@@ -177,7 +202,7 @@ static void process_rx_callbacks(void *v_reg, void *v_rx)
rx_reg->cb(rx_reg->user_data, &rx->info, rx->data, rx->len);
}

-static void process_rx(struct mesh_io_private *pvt, int8_t rssi,
+static void process_rx(uint16_t index, struct mesh_io_private *pvt, int8_t rssi,
uint32_t instant, const uint8_t *addr,
const uint8_t *data, uint8_t len)
{
@@ -191,6 +216,10 @@ static void process_rx(struct mesh_io_private *pvt, int8_t rssi,
.info.rssi = rssi,
};

+ /* Accept all traffic except beacons from any controller */
+ if (index != pvt->send_idx && data[0] == MESH_AD_TYPE_BEACON)
+ return;
+
print_packet("RX", data, len);
l_queue_foreach(pvt->rx_regs, process_rx_callbacks, &rx);
}
@@ -205,7 +234,7 @@ static void event_device_found(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
const struct mgmt_ev_mesh_device_found *ev = param;
- struct mesh_io *io = user_data;
+ struct mesh_io_private *pvt = user_data;
const uint8_t *adv;
const uint8_t *addr;
uint32_t instant;
@@ -236,9 +265,10 @@ static void event_device_found(uint16_t index, uint16_t length,
if (len > adv_len)
break;

- if (adv[1] >= 0x29 && adv[1] <= 0x2B)
- process_rx(io->pvt, ev->rssi, instant, addr, adv + 1,
- adv[0]);
+ if (adv[1] >= MESH_AD_TYPE_PROVISION &&
+ adv[1] <= MESH_AD_TYPE_BEACON)
+ process_rx(index, pvt, ev->rssi, instant, addr,
+ adv + 1, adv[0]);

adv += field_len + 1;
}
@@ -320,6 +350,12 @@ static void ctl_up(uint8_t status, uint16_t length,
mesh->num_ad_types = sizeof(mesh_ad_types);
memcpy(mesh->ad_types, mesh_ad_types, sizeof(mesh_ad_types));

+ pvt->rx_id = mesh_mgmt_register(MGMT_EV_MESH_DEVICE_FOUND,
+ MGMT_INDEX_NONE, event_device_found, pvt,
+ NULL);
+ pvt->tx_id = mesh_mgmt_register(MGMT_EV_MESH_PACKET_CMPLT,
+ index, send_cmplt, pvt, NULL);
+
mesh_mgmt_send(MGMT_OP_SET_MESH_RECEIVER, index, len, mesh,
mesh_up, L_UINT_TO_PTR(index), NULL);
l_debug("done %d mesh startup", index);
@@ -407,11 +443,6 @@ static bool dev_init(struct mesh_io *io, void *opts, void *user_data)
mesh_mgmt_send(MGMT_OP_READ_INFO, index, 0, NULL,
read_info_cb, L_UINT_TO_PTR(index), NULL);

- pvt->rx_id = mesh_mgmt_register(MGMT_EV_MESH_DEVICE_FOUND,
- MGMT_INDEX_NONE, event_device_found, io, NULL);
- pvt->tx_id = mesh_mgmt_register(MGMT_EV_MESH_PACKET_CMPLT,
- MGMT_INDEX_NONE, send_cmplt, io, NULL);
-
pvt->dup_filters = l_queue_new();
pvt->rx_regs = l_queue_new();
pvt->tx_pkts = l_queue_new();
@@ -522,6 +553,11 @@ static void send_pkt(struct mesh_io_private *pvt, struct tx_pkt *tx,
send->adv_data_len = tx->len + 1;
send->adv_data[0] = tx->len;
memcpy(send->adv_data + 1, tx->pkt, tx->len);
+
+ /* Filter looped back Provision packets */
+ if (tx->pkt[0] == MESH_AD_TYPE_PROVISION)
+ filter_dups(NULL, send->adv_data, get_instant());
+
mesh_mgmt_send(MGMT_OP_MESH_SEND, index,
len, send, send_queued, tx, NULL);
print_packet("Mesh Send Start", tx->pkt, tx->len);
--
2.39.2



2023-03-16 01:16:38

by Brian Gix

[permalink] [raw]
Subject: [PATCH BlueZ v2 2/3] mesh: Make MGMT mesh-io less noisy

Remove excessive logging traffic
---
mesh/mesh-io-mgmt.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mesh/mesh-io-mgmt.c b/mesh/mesh-io-mgmt.c
index 65994f058..f1385edcc 100644
--- a/mesh/mesh-io-mgmt.c
+++ b/mesh/mesh-io-mgmt.c
@@ -227,7 +227,7 @@ static void process_rx(uint16_t index, struct mesh_io_private *pvt, int8_t rssi,
static void send_cmplt(uint16_t index, uint16_t length,
const void *param, void *user_data)
{
- print_packet("Mesh Send Complete", param, length);
+ /* print_packet("Mesh Send Complete", param, length); */
}

static void event_device_found(uint16_t index, uint16_t length,
@@ -506,7 +506,7 @@ static void send_cancel(struct mesh_io_private *pvt)

if (pvt->handle) {
remove.handle = pvt->handle;
- l_debug("Cancel TX");
+ /* l_debug("Cancel TX"); */
mesh_mgmt_send(MGMT_OP_MESH_SEND_CANCEL, pvt->send_idx,
sizeof(remove), &remove,
NULL, NULL, NULL);
@@ -560,7 +560,7 @@ static void send_pkt(struct mesh_io_private *pvt, struct tx_pkt *tx,

mesh_mgmt_send(MGMT_OP_MESH_SEND, index,
len, send, send_queued, tx, NULL);
- print_packet("Mesh Send Start", tx->pkt, tx->len);
+ /* print_packet("Mesh Send Start", tx->pkt, tx->len); */
pvt->tx = tx;
}

--
2.39.2


2023-03-16 01:16:40

by Brian Gix

[permalink] [raw]
Subject: [PATCH BlueZ v2 3/3] mesh: Don't send Prov Failed on non-existant links

If remote device does not respond to a Prov Link Open request, then the
callbacks do not get established, and attempting to send Failure
messages on the non-existent link rersult in seg fault.
---
mesh/remprv-server.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mesh/remprv-server.c b/mesh/remprv-server.c
index 6a9efdd47..927dbab0e 100644
--- a/mesh/remprv-server.c
+++ b/mesh/remprv-server.c
@@ -546,7 +546,7 @@ static bool remprv_srv_pkt(uint16_t src, uint16_t unicast, uint16_t app_idx,
bool segmented = false;
uint32_t opcode;
uint8_t msg[69];
- uint8_t status;
+ uint8_t old_state, status;
uint16_t n;

if (app_idx != APP_IDX_DEV_LOCAL)
@@ -843,10 +843,12 @@ static bool remprv_srv_pkt(uint16_t src, uint16_t unicast, uint16_t app_idx,
if (!prov || prov->node != node || prov->client != src)
return true;

+ old_state = prov->state;
prov->state = PB_REMOTE_STATE_LINK_CLOSING;
mesh_io_send_cancel(NULL, &pkt_filter, sizeof(pkt_filter));
send_prov_status(prov, PB_REM_ERR_SUCCESS);
- if (pkt[0] == 0x02) {
+ if (pkt[0] == 0x02 &&
+ old_state >= PB_REMOTE_STATE_LINK_ACTIVE) {
msg[0] = PROV_FAILED;
msg[1] = PROV_ERR_CANT_ASSIGN_ADDR;
if (prov->nppi_proc == RPR_ADV)
--
2.39.2


2023-03-16 03:50:08

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ,v2,1/3] mesh: Filter originated Provisioning Data packets

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=730563

---Test result---

Test Summary:
CheckPatch PASS 1.14 seconds
GitLint PASS 0.76 seconds
BuildEll PASS 27.50 seconds
BluezMake PASS 838.66 seconds
MakeCheck PASS 11.01 seconds
MakeDistcheck PASS 152.90 seconds
CheckValgrind PASS 244.64 seconds
CheckSmatch WARNING 320.77 seconds
bluezmakeextell PASS 99.34 seconds
IncrementalBuild PASS 1957.65 seconds
ScanBuild PASS 1036.46 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
mesh/mesh-io-mgmt.c:537:67: warning: Variable length array is used.mesh/mesh-io-mgmt.c:537:67: warning: Variable length array is used.


---
Regards,
Linux Bluetooth

2023-03-16 05:40:22

by patchwork-bot+bluetooth

[permalink] [raw]
Subject: Re: [PATCH BlueZ v2 1/3] mesh: Filter originated Provisioning Data packets

Hello:

This series was applied to bluetooth/bluez.git (master)
by Brian Gix <[email protected]>:

On Wed, 15 Mar 2023 18:16:25 -0700 you wrote:
> The mesh daemon can process incoming mesh packets on more than one
> controller, but if a Provisioning data packet that originated from the
> local daemon is received by a different controller, it must be filtered
> and disregarded, or it will break the provisioning protocol.
> ---
> mesh/mesh-io-mgmt.c | 58 ++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 11 deletions(-)

Here is the summary with links:
- [BlueZ,v2,1/3] mesh: Filter originated Provisioning Data packets
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a5998b588c86
- [BlueZ,v2,2/3] mesh: Make MGMT mesh-io less noisy
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1f7aed635f3
- [BlueZ,v2,3/3] mesh: Don't send Prov Failed on non-existant links
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=806230e4fda7

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html