public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets
@ 2025-07-11 21:57 Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB Christian Eggers
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

The first 3 patches replace further magic numbers by preprocessor 
defines (no functional changes). Patch 4 tries to improve readabilty,
while 5 and 6 are only cleanups (these patches have already been
submitted in my previous series, but were not merged without any
notice).

Patch 7 and 8 are further simplifications/cleanups, while the last patch
fixes a serious bug which causes corruption in all relayed packets. Maybe
this NEVER worked correctly, although this is a major feature of 
Bluetooth Mesh. At leat this one should get reviewied by a Mesh expert.

v2:
- Fix declaration after statement error in patch 9


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 23:26   ` mesh: cleanups and a bugfix for relay packets bluez.test.bot
  2025-07-11 21:57 ` [PATCH BlueZ v2 2/9] mesh: introduce MESH_AD_MAX_LEN and MESH_NET_MAX_PDU_LEN Christian Eggers
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

---
 mesh/net-keys.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index a10f93ccb03e..22ab5b626a84 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -442,10 +442,10 @@ static bool match_beacon(const void *a, const void *b)
 	const uint8_t *incoming = b;
 
 	if (incoming[0] == BEACON_TYPE_MPB)
-		return !memcmp(cached->data, incoming, 27);
+		return !memcmp(cached->data, incoming, BEACON_LEN_MPB - 1);
 
 	if (incoming[0] == BEACON_TYPE_SNB)
-		return !memcmp(cached->data, incoming, 22);
+		return !memcmp(cached->data, incoming, BEACON_LEN_SNB - 1);
 
 	return false;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 2/9] mesh: introduce MESH_AD_MAX_LEN and MESH_NET_MAX_PDU_LEN
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 3/9] mesh: replace MESH internal defines by shared ones Christian Eggers
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

Use symbolic names rather than magic numbers. Remove unneeded extra
pointer in send_seg().
---
 mesh/mesh-defs.h       |  9 +++++++++
 mesh/mesh-io-generic.c |  3 ++-
 mesh/mesh-io-mgmt.c    |  3 ++-
 mesh/mesh-io-unit.c    |  4 +++-
 mesh/net-keys.c        |  6 ++++--
 mesh/net.c             | 17 +++++++++--------
 6 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
index 25ce0126c708..a12acaf59453 100644
--- a/mesh/mesh-defs.h
+++ b/mesh/mesh-defs.h
@@ -12,6 +12,15 @@
 #define MESH_AD_TYPE_NETWORK	0x2A
 #define MESH_AD_TYPE_BEACON	0x2B
 
+/*
+ * MshPRT_v1.1, section 3.3.1 / Core_v5.3, section 2.3.1.3
+ * Maximum length of AdvData without 'Length' field (30)
+ */
+#define MESH_AD_MAX_LEN		(BT_AD_MAX_DATA_LEN - 1)
+
+/* Max size of a Network PDU, prior prepending AD type (29)*/
+#define MESH_NET_MAX_PDU_LEN	(MESH_AD_MAX_LEN - 1)
+
 #define FEATURE_RELAY	1
 #define FEATURE_PROXY	2
 #define FEATURE_FRIEND	4
diff --git a/mesh/mesh-io-generic.c b/mesh/mesh-io-generic.c
index 1ec4f379def0..0875a359bd78 100644
--- a/mesh/mesh-io-generic.c
+++ b/mesh/mesh-io-generic.c
@@ -19,6 +19,7 @@
 #include <ell/ell.h>
 
 #include "monitor/bt.h"
+#include "src/shared/ad.h"
 #include "src/shared/hci.h"
 #include "src/shared/mgmt.h"
 #include "lib/bluetooth.h"
@@ -52,7 +53,7 @@ struct tx_pkt {
 	struct mesh_io_send_info	info;
 	bool				delete;
 	uint8_t				len;
-	uint8_t				pkt[30];
+	uint8_t				pkt[MESH_AD_MAX_LEN];
 };
 
 struct tx_pattern {
diff --git a/mesh/mesh-io-mgmt.c b/mesh/mesh-io-mgmt.c
index 4ca7ff93c863..065067fc2821 100644
--- a/mesh/mesh-io-mgmt.c
+++ b/mesh/mesh-io-mgmt.c
@@ -23,6 +23,7 @@
 #include "lib/bluetooth.h"
 #include "lib/bluetooth.h"
 #include "lib/mgmt.h"
+#include "src/shared/ad.h"
 #include "src/shared/mgmt.h"
 
 #include "mesh/mesh-defs.h"
@@ -60,7 +61,7 @@ struct tx_pkt {
 	struct mesh_io_send_info	info;
 	bool				delete;
 	uint8_t				len;
-	uint8_t				pkt[30];
+	uint8_t				pkt[MESH_AD_MAX_LEN];
 };
 
 struct tx_pattern {
diff --git a/mesh/mesh-io-unit.c b/mesh/mesh-io-unit.c
index 936f5a9514c5..f9a5aaa3e210 100644
--- a/mesh/mesh-io-unit.c
+++ b/mesh/mesh-io-unit.c
@@ -22,6 +22,8 @@
 #include <time.h>
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-defs.h"
 #include "mesh/dbus.h"
 #include "mesh/mesh-io.h"
@@ -59,7 +61,7 @@ struct tx_pkt {
 	struct mesh_io_send_info	info;
 	bool				delete;
 	uint8_t				len;
-	uint8_t				pkt[30];
+	uint8_t				pkt[MESH_AD_MAX_LEN];
 };
 
 struct tx_pattern {
diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index 22ab5b626a84..9b11bb7a1da2 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -16,6 +16,8 @@
 
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-defs.h"
 #include "mesh/util.h"
 #include "mesh/crypto.h"
@@ -74,8 +76,8 @@ static struct l_queue *keys;
 static uint32_t last_flooding_id;
 
 /* To avoid re-decrypting same packet for multiple nodes, cache and check */
-static uint8_t cache_pkt[29];
-static uint8_t cache_plain[29];
+static uint8_t cache_pkt[MESH_NET_MAX_PDU_LEN];
+static uint8_t cache_plain[MESH_NET_MAX_PDU_LEN];
 static size_t cache_len;
 static size_t cache_plainlen;
 static uint32_t cache_id;
diff --git a/mesh/net.c b/mesh/net.c
index d711f80c83d1..cf4f337616d5 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -19,6 +19,8 @@
 
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-defs.h"
 #include "mesh/util.h"
 #include "mesh/crypto.h"
@@ -203,7 +205,7 @@ struct oneshot_tx {
 	uint16_t interval;
 	uint8_t cnt;
 	uint8_t size;
-	uint8_t packet[30];
+	uint8_t packet[MESH_AD_MAX_LEN];
 };
 
 struct net_beacon_data {
@@ -2246,7 +2248,7 @@ static bool match_by_dst(const void *a, const void *b)
 
 static void send_relay_pkt(struct mesh_net *net, uint8_t *data, uint8_t size)
 {
-	uint8_t packet[30];
+	uint8_t packet[MESH_AD_MAX_LEN];
 	struct mesh_io *io = net->io;
 	struct mesh_io_send_info info = {
 		.type = MESH_IO_TIMING_TYPE_GENERAL,
@@ -3130,8 +3132,7 @@ static bool send_seg(struct mesh_net *net, uint8_t cnt, uint16_t interval,
 {
 	struct mesh_subnet *subnet;
 	uint8_t seg_len;
-	uint8_t gatt_data[30];
-	uint8_t *packet = gatt_data;
+	uint8_t packet[MESH_AD_MAX_LEN];
 	uint8_t packet_len;
 	uint8_t segN = SEG_MAX(msg->segmented, msg->len);
 	uint16_t seg_off = SEG_OFF(segO);
@@ -3193,7 +3194,7 @@ void mesh_net_send_seg(struct mesh_net *net, uint32_t net_key_id,
 			uint16_t src, uint16_t dst, uint32_t hdr,
 			const void *seg, uint16_t seg_len)
 {
-	uint8_t packet[30];
+	uint8_t packet[MESH_AD_MAX_LEN];
 	uint8_t packet_len;
 	bool segmented = !!((hdr >> SEG_HDR_SHIFT) & 0x1);
 	uint8_t key_aid = (hdr >> KEY_HDR_SHIFT) & KEY_ID_MASK;
@@ -3353,7 +3354,7 @@ void mesh_net_ack_send(struct mesh_net *net, uint32_t net_key_id,
 	uint32_t hdr;
 	uint8_t data[7];
 	uint8_t pkt_len;
-	uint8_t pkt[30];
+	uint8_t pkt[MESH_AD_MAX_LEN];
 
 	/*
 	 * MshPRFv1.0.1 section 3.4.5.2, Interface output filter:
@@ -3400,7 +3401,7 @@ void mesh_net_transport_send(struct mesh_net *net, uint32_t net_key_id,
 				uint16_t msg_len)
 {
 	uint8_t pkt_len;
-	uint8_t pkt[30];
+	uint8_t pkt[MESH_AD_MAX_LEN];
 	bool result = false;
 
 	if (!net->src_addr)
@@ -3416,7 +3417,7 @@ void mesh_net_transport_send(struct mesh_net *net, uint32_t net_key_id,
 		ttl = net->default_ttl;
 
 	/* Range check the Opcode and msg length*/
-	if (*msg & 0xc0 || (9 + msg_len + 8 > 29))
+	if (*msg & 0xc0 || (9 + msg_len + 8 > MESH_NET_MAX_PDU_LEN))
 		return;
 
 	/*
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 3/9] mesh: replace MESH internal defines by shared ones
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 2/9] mesh: introduce MESH_AD_MAX_LEN and MESH_NET_MAX_PDU_LEN Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 4/9] mesh: net: constify tx path Christian Eggers
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

Replace BT_AD_MESH_* by MESH_AD_TYPE_*.

Both sets of definition have been added almost at the time, so maybe
it was a 'race condition'.
---
 mesh/manager.c         |  4 +++-
 mesh/mesh-defs.h       |  4 ----
 mesh/mesh-io-generic.c |  4 ++--
 mesh/mesh-io-mgmt.c    | 17 ++++++++---------
 mesh/mesh-io.c         |  3 ++-
 mesh/mesh.c            |  6 ++++--
 mesh/net-keys.c        |  4 ++--
 mesh/net.c             | 16 ++++++++--------
 mesh/pb-adv.c          | 14 ++++++++------
 mesh/prov-acceptor.c   |  8 ++++----
 mesh/prov-initiator.c  |  3 ++-
 11 files changed, 43 insertions(+), 40 deletions(-)

diff --git a/mesh/manager.c b/mesh/manager.c
index 3786f7a8f4cd..b69866355bd1 100644
--- a/mesh/manager.c
+++ b/mesh/manager.c
@@ -17,6 +17,8 @@
 #define _GNU_SOURCE
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-defs.h"
 #include "mesh/dbus.h"
 #include "mesh/error.h"
@@ -57,7 +59,7 @@ struct scan_req {
 
 static struct l_queue *scans;
 static struct prov_remote_data *prov_pending;
-static const uint8_t prvb[2] = {MESH_AD_TYPE_BEACON, 0x00};
+static const uint8_t prvb[2] = {BT_AD_MESH_BEACON, 0x00};
 
 static bool by_scan(const void *a, const void *b)
 {
diff --git a/mesh/mesh-defs.h b/mesh/mesh-defs.h
index a12acaf59453..5b0403d1315e 100644
--- a/mesh/mesh-defs.h
+++ b/mesh/mesh-defs.h
@@ -8,10 +8,6 @@
  *
  */
 
-#define MESH_AD_TYPE_PROVISION	0x29
-#define MESH_AD_TYPE_NETWORK	0x2A
-#define MESH_AD_TYPE_BEACON	0x2B
-
 /*
  * MshPRT_v1.1, section 3.3.1 / Core_v5.3, section 2.3.1.3
  * Maximum length of AdvData without 'Length' field (30)
diff --git a/mesh/mesh-io-generic.c b/mesh/mesh-io-generic.c
index 0875a359bd78..f65de9d8d6de 100644
--- a/mesh/mesh-io-generic.c
+++ b/mesh/mesh-io-generic.c
@@ -353,8 +353,8 @@ static bool find_active(const void *a, const void *b)
 	/* Mesh specific AD types do *not* require active scanning,
 	 * so do not turn on Active Scanning on their account.
 	 */
-	if (rx_reg->filter[0] < MESH_AD_TYPE_PROVISION ||
-			rx_reg->filter[0] > MESH_AD_TYPE_BEACON)
+	if (rx_reg->filter[0] < BT_AD_MESH_PROV ||
+			rx_reg->filter[0] > BT_AD_MESH_BEACON)
 		return true;
 
 	return false;
diff --git a/mesh/mesh-io-mgmt.c b/mesh/mesh-io-mgmt.c
index 065067fc2821..30d3981bc14f 100644
--- a/mesh/mesh-io-mgmt.c
+++ b/mesh/mesh-io-mgmt.c
@@ -156,7 +156,7 @@ static bool filter_dups(const uint8_t *addr, const uint8_t *adv,
 	if (!addr)
 		addr = zero_addr;
 
-	if (adv[1] == MESH_AD_TYPE_PROVISION) {
+	if (adv[1] == BT_AD_MESH_PROV) {
 		filter = l_queue_find(pvt->dup_filters, find_by_adv, adv);
 
 		if (!filter && addr != zero_addr)
@@ -215,7 +215,7 @@ static void process_rx(uint16_t index, struct mesh_io_private *pvt, int8_t rssi,
 	};
 
 	/* Accept all traffic except beacons from any controller */
-	if (index != pvt->send_idx && data[0] == MESH_AD_TYPE_BEACON)
+	if (index != pvt->send_idx && data[0] == BT_AD_MESH_BEACON)
 		return;
 
 	print_packet("RX", data, len);
@@ -263,8 +263,7 @@ static void event_device_found(uint16_t index, uint16_t length,
 		if (len > adv_len)
 			break;
 
-		if (adv[1] >= MESH_AD_TYPE_PROVISION &&
-					adv[1] <= MESH_AD_TYPE_BEACON)
+		if (adv[1] >= BT_AD_MESH_PROV && adv[1] <= BT_AD_MESH_BEACON)
 			process_rx(index, pvt, ev->rssi, instant, addr,
 							adv + 1, adv[0]);
 
@@ -303,8 +302,8 @@ static bool find_active(const void *a, const void *b)
 	/* Mesh specific AD types do *not* require active scanning,
 	 * so do not turn on Active Scanning on their account.
 	 */
-	if (rx_reg->filter[0] < MESH_AD_TYPE_PROVISION ||
-			rx_reg->filter[0] > MESH_AD_TYPE_BEACON)
+	if (rx_reg->filter[0] < BT_AD_MESH_PROV ||
+					rx_reg->filter[0] > BT_AD_MESH_BEACON)
 		return true;
 
 	return false;
@@ -332,8 +331,8 @@ static void ctl_up(uint8_t status, uint16_t length,
 	int index = L_PTR_TO_UINT(user_data);
 	uint16_t len;
 	struct mgmt_cp_set_mesh *mesh;
-	uint8_t mesh_ad_types[] = { MESH_AD_TYPE_NETWORK,
-				MESH_AD_TYPE_BEACON, MESH_AD_TYPE_PROVISION };
+	uint8_t mesh_ad_types[] = { BT_AD_MESH_DATA, BT_AD_MESH_BEACON,
+							BT_AD_MESH_PROV };
 
 	l_debug("HCI%d is up status: %d", index, status);
 	if (status)
@@ -544,7 +543,7 @@ static void send_pkt(struct mesh_io_private *pvt, struct tx_pkt *tx,
 	memcpy(send->adv_data + 1, tx->pkt, tx->len);
 
 	/* Filter looped back Provision packets */
-	if (tx->pkt[0] == MESH_AD_TYPE_PROVISION)
+	if (tx->pkt[0] == BT_AD_MESH_PROV)
 		filter_dups(NULL, send->adv_data, get_instant());
 
 	mesh_mgmt_send(MGMT_OP_MESH_SEND, index,
diff --git a/mesh/mesh-io.c b/mesh/mesh-io.c
index 99c7c2014375..ec5feb9c2714 100644
--- a/mesh/mesh-io.c
+++ b/mesh/mesh-io.c
@@ -18,6 +18,7 @@
 
 #include "lib/bluetooth.h"
 #include "lib/mgmt.h"
+#include "src/shared/ad.h"
 #include "src/shared/mgmt.h"
 
 #include "mesh/mesh-defs.h"
@@ -42,7 +43,7 @@ static const struct mesh_io_table table[] = {
 	{MESH_IO_TYPE_UNIT_TEST, &mesh_io_unit},
 };
 
-static const uint8_t unprv_filter[] = { MESH_AD_TYPE_BEACON, 0 };
+static const uint8_t unprv_filter[] = { BT_AD_MESH_BEACON, 0 };
 
 static struct mesh_io *default_io;
 static struct l_timeout *loop_adv_to;
diff --git a/mesh/mesh.c b/mesh/mesh.c
index 01a1607b1a31..db77602d37da 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -17,6 +17,8 @@
 #define _GNU_SOURCE
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-io.h"
 #include "mesh/node.h"
 #include "mesh/net.h"
@@ -139,7 +141,7 @@ static void prov_rx(void *user_data, struct mesh_io_recv_info *info,
 
 bool mesh_reg_prov_rx(prov_rx_cb_t cb, void *user_data)
 {
-	uint8_t prov_filter[] = {MESH_AD_TYPE_PROVISION};
+	uint8_t prov_filter[] = {BT_AD_MESH_PROV};
 
 	if (mesh.prov_rx && mesh.prov_rx != cb)
 		return false;
@@ -153,7 +155,7 @@ bool mesh_reg_prov_rx(prov_rx_cb_t cb, void *user_data)
 
 void mesh_unreg_prov_rx(prov_rx_cb_t cb)
 {
-	uint8_t prov_filter[] = {MESH_AD_TYPE_PROVISION};
+	uint8_t prov_filter[] = {BT_AD_MESH_PROV};
 
 	if (mesh.prov_rx != cb)
 		return;
diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index 9b11bb7a1da2..338d287a7ef7 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -399,7 +399,7 @@ static bool mpb_compose(struct net_key *key, uint32_t ivi, bool kr, bool ivu)
 						b_data, 5, b_data, NULL, 8))
 		return false;
 
-	key->mpb[0] = MESH_AD_TYPE_BEACON;
+	key->mpb[0] = BT_AD_MESH_BEACON;
 	key->mpb[1] = BEACON_TYPE_MPB;
 	memcpy(key->mpb + 2, random, 13);
 	memcpy(key->mpb + 15, b_data, 13);
@@ -421,7 +421,7 @@ static bool snb_compose(struct net_key *key, uint32_t ivi, bool kr, bool ivu)
 		return false;
 	}
 
-	key->snb[0] = MESH_AD_TYPE_BEACON;
+	key->snb[0] = BT_AD_MESH_BEACON;
 	key->snb[1] = BEACON_TYPE_SNB;
 	key->snb[2] = 0;
 
diff --git a/mesh/net.c b/mesh/net.c
index cf4f337616d5..b6ff11ffd777 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2258,7 +2258,7 @@ static void send_relay_pkt(struct mesh_net *net, uint8_t *data, uint8_t size)
 		.u.gen.max_delay = DEFAULT_MAX_DELAY
 	};
 
-	packet[0] = MESH_AD_TYPE_NETWORK;
+	packet[0] = BT_AD_MESH_DATA;
 	memcpy(packet + 1, data, size);
 
 	mesh_io_send(io, &info, packet, size + 1);
@@ -2292,7 +2292,7 @@ static void send_msg_pkt_oneshot(void *user_data)
 		return;
 	}
 
-	tx->packet[0] = MESH_AD_TYPE_NETWORK;
+	tx->packet[0] = BT_AD_MESH_DATA;
 	info.type = MESH_IO_TIMING_TYPE_GENERAL;
 	info.u.gen.interval = tx->interval;
 	info.u.gen.cnt = tx->cnt;
@@ -3003,9 +3003,9 @@ bool mesh_net_attach(struct mesh_net *net, struct mesh_io *io)
 
 	first = l_queue_isempty(nets);
 	if (first) {
-		const uint8_t snb[] = {MESH_AD_TYPE_BEACON, 1};
-		const uint8_t mpb[] = {MESH_AD_TYPE_BEACON, 2};
-		const uint8_t pkt[] = {MESH_AD_TYPE_NETWORK};
+		const uint8_t snb[] = {BT_AD_MESH_BEACON, 1};
+		const uint8_t mpb[] = {BT_AD_MESH_BEACON, 2};
+		const uint8_t pkt[] = {BT_AD_MESH_DATA};
 
 		if (!nets)
 			nets = l_queue_new();
@@ -3033,9 +3033,9 @@ bool mesh_net_attach(struct mesh_net *net, struct mesh_io *io)
 
 struct mesh_io *mesh_net_detach(struct mesh_net *net)
 {
-	const uint8_t snb[] = {MESH_AD_TYPE_BEACON, 1};
-	const uint8_t mpb[] = {MESH_AD_TYPE_BEACON, 2};
-	const uint8_t pkt[] = {MESH_AD_TYPE_NETWORK};
+	const uint8_t snb[] = {BT_AD_MESH_BEACON, 1};
+	const uint8_t mpb[] = {BT_AD_MESH_BEACON, 2};
+	const uint8_t pkt[] = {BT_AD_MESH_DATA};
 	struct mesh_io *io;
 	uint8_t type = 0;
 
diff --git a/mesh/pb-adv.c b/mesh/pb-adv.c
index 0b1fd7d577ff..1b80b97ad31c 100644
--- a/mesh/pb-adv.c
+++ b/mesh/pb-adv.c
@@ -16,6 +16,8 @@
 
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
+
 #include "mesh/mesh-defs.h"
 #include "mesh/crypto.h"
 #include "mesh/net.h"
@@ -97,7 +99,7 @@ struct idle_rx {
 
 static struct l_queue *pb_sessions = NULL;
 
-static const uint8_t filter[1] = { MESH_AD_TYPE_PROVISION };
+static const uint8_t filter[1] = { BT_AD_MESH_PROV };
 
 static void pb_adv_packet(void *user_data, const uint8_t *pkt, uint16_t len);
 
@@ -130,7 +132,7 @@ static void send_adv_segs(struct pb_adv_session *session, const uint8_t *data,
 							uint16_t size)
 {
 	uint16_t init_size;
-	uint8_t buf[PB_ADV_MTU + 6] = { MESH_AD_TYPE_PROVISION };
+	uint8_t buf[PB_ADV_MTU + 6] = { BT_AD_MESH_PROV };
 	uint8_t max_seg;
 	uint8_t consumed;
 	int i;
@@ -236,7 +238,7 @@ static void pb_adv_tx(void *user_data, const void *data, uint16_t len)
 
 static void send_open_req(struct pb_adv_session *session)
 {
-	struct pb_open_req open_req = { MESH_AD_TYPE_PROVISION };
+	struct pb_open_req open_req = { BT_AD_MESH_PROV };
 
 	l_put_be32(session->link_id, &open_req.link_id);
 	open_req.trans_num = 0;
@@ -251,7 +253,7 @@ static void send_open_req(struct pb_adv_session *session)
 
 static void send_open_cfm(struct pb_adv_session *session)
 {
-	struct pb_open_cfm open_cfm = { MESH_AD_TYPE_PROVISION };
+	struct pb_open_cfm open_cfm = { BT_AD_MESH_PROV };
 
 	l_put_be32(session->link_id, &open_cfm.link_id);
 	open_cfm.trans_num = 0;
@@ -265,7 +267,7 @@ static void send_open_cfm(struct pb_adv_session *session)
 
 static void send_ack(struct pb_adv_session *session, uint8_t trans_num)
 {
-	struct pb_ack ack = { MESH_AD_TYPE_PROVISION };
+	struct pb_ack ack = { BT_AD_MESH_PROV };
 
 	if (!l_queue_find(pb_sessions, session_match, session))
 		return;
@@ -280,7 +282,7 @@ static void send_ack(struct pb_adv_session *session, uint8_t trans_num)
 
 static void send_close_ind(struct pb_adv_session *session, uint8_t reason)
 {
-	struct pb_close_ind close_ind = { MESH_AD_TYPE_PROVISION };
+	struct pb_close_ind close_ind = { BT_AD_MESH_PROV };
 
 	if (!l_queue_find(pb_sessions, session_match, session))
 		return;
diff --git a/mesh/prov-acceptor.c b/mesh/prov-acceptor.c
index 650309b635cd..0cedc227ad28 100644
--- a/mesh/prov-acceptor.c
+++ b/mesh/prov-acceptor.c
@@ -16,6 +16,7 @@
 
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
 #include "src/shared/ecc.h"
 
 #include "mesh/mesh-defs.h"
@@ -50,8 +51,8 @@ struct deferred_cmd {
 	uint8_t cmd[];
 };
 
-static const uint8_t pkt_filter = MESH_AD_TYPE_PROVISION;
-static const uint8_t bec_filter[] = {MESH_AD_TYPE_BEACON,
+static const uint8_t pkt_filter = BT_AD_MESH_PROV;
+static const uint8_t bec_filter[] = {BT_AD_MESH_BEACON,
 						BEACON_TYPE_UNPROVISIONED};
 
 #define MAT_REMOTE_PUBLIC	0x01
@@ -736,8 +737,7 @@ bool acceptor_start(uint8_t num_ele, uint8_t *uuid,
 		void *caller_data)
 {
 	struct mesh_agent_prov_caps *caps;
-	uint8_t beacon[24] = {MESH_AD_TYPE_BEACON,
-						BEACON_TYPE_UNPROVISIONED};
+	uint8_t beacon[24] = {BT_AD_MESH_BEACON, BEACON_TYPE_UNPROVISIONED};
 	uint8_t len = sizeof(beacon) - sizeof(uint32_t);
 	bool result;
 
diff --git a/mesh/prov-initiator.c b/mesh/prov-initiator.c
index dc19d1e9b7a5..c0d2de443ac1 100644
--- a/mesh/prov-initiator.c
+++ b/mesh/prov-initiator.c
@@ -16,6 +16,7 @@
 
 #include <ell/ell.h>
 
+#include "src/shared/ad.h"
 #include "src/shared/ecc.h"
 
 #include "mesh/mesh-defs.h"
@@ -51,7 +52,7 @@ static const uint16_t expected_pdu_size[] = {
 
 #define BEACON_TYPE_UNPROVISIONED		0x00
 
-static const uint8_t pkt_filter = MESH_AD_TYPE_PROVISION;
+static const uint8_t pkt_filter = BT_AD_MESH_PROV;
 
 enum int_state {
 	INT_PROV_IDLE = 0,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 4/9] mesh: net: constify tx path
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (2 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 3/9] mesh: replace MESH internal defines by shared ones Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 5/9] mesh: net: remove unused stuff Christian Eggers
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

Although the first byte of network packets has the same value for all 4
'send' functions, it feels much more natural to assign this byte at the
location(s) where the packet is assembled, rather than where it is sent.
This improves the readability because send_msg_pkt() isn't called with a
partially uninitialized buffer anymore.
---
 mesh/net.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mesh/net.c b/mesh/net.c
index b6ff11ffd777..496e4dd7fc04 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2292,7 +2292,6 @@ static void send_msg_pkt_oneshot(void *user_data)
 		return;
 	}
 
-	tx->packet[0] = BT_AD_MESH_DATA;
 	info.type = MESH_IO_TIMING_TYPE_GENERAL;
 	info.u.gen.interval = tx->interval;
 	info.u.gen.cnt = tx->cnt;
@@ -2305,7 +2304,7 @@ static void send_msg_pkt_oneshot(void *user_data)
 }
 
 static void send_msg_pkt(struct mesh_net *net, uint8_t cnt, uint16_t interval,
-						uint8_t *packet, uint8_t size)
+					const uint8_t *packet, uint8_t size)
 {
 	struct oneshot_tx *tx = l_new(struct oneshot_tx, 1);
 
@@ -3159,6 +3158,7 @@ static bool send_seg(struct mesh_net *net, uint8_t cnt, uint16_t interval,
 	l_debug("segN %d segment %d seg_off %d", segN, segO, seg_off);
 
 	/* TODO: Are we RXing on an LPN's behalf? Then set RLY bit */
+	packet[0] = BT_AD_MESH_DATA;
 	if (!mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src,
 					msg->remote, 0, msg->segmented,
 					msg->key_aid, msg->szmic,
@@ -3216,6 +3216,7 @@ void mesh_net_send_seg(struct mesh_net *net, uint32_t net_key_id,
 	l_debug("SEQ0: %6.6x", seq);
 	l_debug("segO: %d", segO);
 
+	packet[0] = BT_AD_MESH_DATA;
 	if (!mesh_crypto_packet_build(false, ttl, seq, src, dst, 0,
 					segmented, key_aid, szmic,
 					seqZero, segO, segN, seg, seg_len,
@@ -3370,6 +3371,7 @@ void mesh_net_ack_send(struct mesh_net *net, uint32_t net_key_id,
 	l_put_be32(ack_flags, data + 3);
 
 	/* Not Segmented, no Key ID associated, no segO or segN */
+	pkt[0] = BT_AD_MESH_DATA;
 	if (!mesh_crypto_packet_build(true, ttl, seq, src, dst,
 					NET_OP_SEG_ACKNOWLEDGE, false, 0, false,
 					seqZero, 0, 0, data + 1, 6,
@@ -3457,6 +3459,7 @@ void mesh_net_transport_send(struct mesh_net *net, uint32_t net_key_id,
 			return;
 	}
 
+	pkt[0] = BT_AD_MESH_DATA;
 	if (!mesh_crypto_packet_build(true, ttl, seq, src, dst, msg[0],
 				false, 0, false, 0, 0, 0, msg + 1, msg_len - 1,
 				pkt + 1, &pkt_len))
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 5/9] mesh: net: remove unused stuff
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (3 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 4/9] mesh: net: constify tx path Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 6/9] mesh: net: update comment Christian Eggers
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

'struct mesh_key_set' and 'struct net_decode' arent't used anymore since
commit 994932b740c7 ("mesh: Refactor friend.c and net.c for central key
DB").

'mesh_status_func_t' isn't used anymore since commit c4bf0626fb62
("mesh: Replace storage_save_config with mesh_config_save_config").
---
 mesh/net.c | 11 -----------
 mesh/net.h |  9 ---------
 2 files changed, 20 deletions(-)

diff --git a/mesh/net.c b/mesh/net.c
index 496e4dd7fc04..3a93f6624e8c 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -176,17 +176,6 @@ struct mesh_destination {
 	uint16_t ref_cnt;
 };
 
-struct net_decode {
-	struct mesh_net *net;
-	struct mesh_friend *frnd;
-	struct mesh_key_set *key_set;
-	uint8_t *packet;
-	uint32_t iv_index;
-	uint8_t size;
-	uint8_t nid;
-	bool proxy;
-};
-
 struct net_queue_data {
 	struct mesh_io_recv_info *info;
 	struct mesh_net *net;
diff --git a/mesh/net.h b/mesh/net.h
index bdb797e1249b..5200beb2fada 100644
--- a/mesh/net.h
+++ b/mesh/net.h
@@ -143,13 +143,6 @@ struct mesh_net_heartbeat_pub {
 	uint8_t ttl;
 };
 
-struct mesh_key_set {
-	bool frnd;
-	uint8_t nid;
-	uint8_t enc_key[16];
-	uint8_t privacy_key[16];
-};
-
 struct friend_neg {
 	int8_t rssi;
 	bool clearing;
@@ -219,8 +212,6 @@ struct mesh_friend_msg {
 	} u;
 };
 
-typedef void (*mesh_status_func_t)(void *user_data, bool result);
-
 struct mesh_net *mesh_net_new(struct mesh_node *node);
 void mesh_net_free(void *net);
 void mesh_net_cleanup(void);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 6/9] mesh: net: update comment
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (4 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 5/9] mesh: net: remove unused stuff Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 7/9] mesh: crypto: mesh_crypto_aes_ccm_encrypt(): remove unused parameter Christian Eggers
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

term 'master credentials' has been replaced by 'flooding credentials' in
commit 09f87c80f1d5 ("mesh: Inclusive language changes")
---
 mesh/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mesh/net.c b/mesh/net.c
index 3a93f6624e8c..b29e24f5d4a9 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2487,7 +2487,7 @@ static void net_rx(void *net_ptr, void *user_data)
 	if (relay_advice > data->relay_advice) {
 		/*
 		 * If packet was encrypted with friendship credentials,
-		 * relay it using master credentials
+		 * relay it using flooding credentials
 		 */
 		if (frnd && !mesh_net_get_key(net, false, net_idx, &net_key_id))
 			return;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 7/9] mesh: crypto: mesh_crypto_aes_ccm_encrypt(): remove unused parameter
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (5 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 6/9] mesh: net: update comment Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 8/9] mesh: crypto: simplify mesh_crypto_packet_parse() Christian Eggers
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

The 'out_mic' pointer isn't written by this function and all callers
pass a NULL pointer for this. It's obviously not required (and would not
work), so lets remove it.
---
 mesh/crypto.c           | 10 ++++------
 mesh/crypto.h           |  3 +--
 mesh/net-keys.c         |  2 +-
 mesh/prov-initiator.c   |  2 +-
 unit/test-mesh-crypto.c |  2 +-
 5 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/mesh/crypto.c b/mesh/crypto.c
index 31001d283a04..3200d1231f1f 100644
--- a/mesh/crypto.c
+++ b/mesh/crypto.c
@@ -84,8 +84,7 @@ bool mesh_crypto_aes_cmac(const uint8_t key[16], const uint8_t *msg,
 bool mesh_crypto_aes_ccm_encrypt(const uint8_t nonce[13], const uint8_t key[16],
 					const uint8_t *aad, uint16_t aad_len,
 					const void *msg, uint16_t msg_len,
-					void *out_msg,
-					void *out_mic, size_t mic_size)
+					void *out_msg, size_t mic_size)
 {
 	void *cipher;
 	bool result;
@@ -733,8 +732,7 @@ bool mesh_crypto_payload_encrypt(uint8_t *aad, const uint8_t *payload,
 	if (!mesh_crypto_aes_ccm_encrypt(nonce, app_key,
 							aad, aad ? 16 : 0,
 							payload, payload_len,
-							out, NULL,
-							aszmic ? 8 : 4))
+							out, aszmic ? 8 : 4))
 		return false;
 
 	return true;
@@ -812,13 +810,13 @@ static bool mesh_crypto_packet_encrypt(uint8_t *packet, uint8_t packet_len,
 		if (!mesh_crypto_aes_ccm_encrypt(nonce, network_key,
 					NULL, 0,
 					packet + 7, packet_len - 7 - 8,
-					packet + 7, NULL, 8))
+					packet + 7, 8))
 			return false;
 	} else {
 		if (!mesh_crypto_aes_ccm_encrypt(nonce, network_key,
 					NULL, 0,
 					packet + 7, packet_len - 7 - 4,
-					packet + 7, NULL, 4))
+					packet + 7, 4))
 			return false;
 	}
 
diff --git a/mesh/crypto.h b/mesh/crypto.h
index 5e4d1d229c19..e4bbe4343223 100644
--- a/mesh/crypto.h
+++ b/mesh/crypto.h
@@ -15,8 +15,7 @@
 bool mesh_crypto_aes_ccm_encrypt(const uint8_t nonce[13], const uint8_t key[16],
 					const uint8_t *aad, uint16_t aad_len,
 					const void *msg, uint16_t msg_len,
-					void *out_msg,
-					void *out_mic, size_t mic_size);
+					void *out_msg, size_t mic_size);
 bool mesh_crypto_aes_ccm_decrypt(const uint8_t nonce[13], const uint8_t key[16],
 				const uint8_t *aad, uint16_t aad_len,
 				const void *enc_msg, uint16_t enc_msg_len,
diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index 338d287a7ef7..0daeb9209b86 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -396,7 +396,7 @@ static bool mpb_compose(struct net_key *key, uint32_t ivi, bool kr, bool ivu)
 
 	l_getrandom(random, sizeof(random));
 	if (!mesh_crypto_aes_ccm_encrypt(random, key->pvt_key, NULL, 0,
-						b_data, 5, b_data, NULL, 8))
+						b_data, 5, b_data, 8))
 		return false;
 
 	key->mpb[0] = BT_AD_MESH_BEACON;
diff --git a/mesh/prov-initiator.c b/mesh/prov-initiator.c
index c0d2de443ac1..d46081c7ae19 100644
--- a/mesh/prov-initiator.c
+++ b/mesh/prov-initiator.c
@@ -466,7 +466,7 @@ void initiator_prov_data(uint16_t net_idx, uint16_t primary, void *caller_data)
 			&prov_data.data,
 			sizeof(prov_data.data),
 			&prov_data.data,
-			NULL, sizeof(prov_data.mic));
+			sizeof(prov_data.mic));
 	print_packet("EncdData", &prov_data.data, sizeof(prov_data) - 1);
 	prov->trans_tx(prov->trans_data, &prov_data, sizeof(prov_data));
 	prov->state = INT_PROV_DATA_SENT;
diff --git a/unit/test-mesh-crypto.c b/unit/test-mesh-crypto.c
index 81f0724fe540..36cae70a68a4 100644
--- a/unit/test-mesh-crypto.c
+++ b/unit/test-mesh-crypto.c
@@ -1864,7 +1864,7 @@ static void check_beacon(const struct mesh_crypto_test *keys)
 		l_put_be32(keys->iv_index, beacon + 15);
 		mesh_crypto_aes_ccm_encrypt(random, enc_key, NULL, 0,
 							beacon + 14, 5,
-							beacon + 14, NULL, 8);
+							beacon + 14, 8);
 		verify_data("BeaconMIC", 0, keys->beacon_cmac, beacon + 19, 8);
 		verify_data("PrivBeacon", 0, keys->beacon, beacon, 27);
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 8/9] mesh: crypto: simplify mesh_crypto_packet_parse()
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (6 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 7/9] mesh: crypto: mesh_crypto_aes_ccm_encrypt(): remove unused parameter Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-11 21:57 ` [PATCH BlueZ v2 9/9] mesh: fix corrupted relay packets Christian Eggers
  2025-07-15 14:30 ` [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for " patchwork-bot+bluetooth
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

- NULL pointer checks are not required (no caller passes NULL pointers)
- reuse result of mesh_crypto_network_header_parse() rather than
  determining value of 'CTL' again.
---
 mesh/crypto.c | 30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

diff --git a/mesh/crypto.c b/mesh/crypto.c
index 3200d1231f1f..3dcf226ad8d3 100644
--- a/mesh/crypto.c
+++ b/mesh/crypto.c
@@ -643,7 +643,7 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 	if (segmented)
 		*segmented = is_segmented;
 
-	if (packet[1] & CTL) {
+	if (*ctl) {
 		uint8_t this_opcode = packet[9] & OPCODE_MASK;
 
 		if (cookie)
@@ -660,17 +660,11 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 				*seqZero = (hdr >> SEQ_ZERO_HDR_SHIFT) &
 								SEQ_ZERO_MASK;
 
-			if (payload)
-				*payload = packet + 9;
-
-			if (payload_len)
-				*payload_len = packet_len - 9;
+			*payload = packet + 9;
+			*payload_len = packet_len - 9;
 		} else {
-			if (payload)
-				*payload = packet + 10;
-
-			if (payload_len)
-				*payload_len = packet_len - 10;
+			*payload = packet + 10;
+			*payload_len = packet_len - 10;
 		}
 	} else {
 		if (cookie)
@@ -693,17 +687,11 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 			if (segN)
 				*segN = (hdr >> SEGN_HDR_SHIFT) & SEG_MASK;
 
-			if (payload)
-				*payload = packet + 13;
-
-			if (payload_len)
-				*payload_len = packet_len - 13;
+			*payload = packet + 13;
+			*payload_len = packet_len - 13;
 		} else {
-			if (payload)
-				*payload = packet + 10;
-
-			if (payload_len)
-				*payload_len = packet_len - 10;
+			*payload = packet + 10;
+			*payload_len = packet_len - 10;
 		}
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH BlueZ v2 9/9] mesh: fix corrupted relay packets
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (7 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 8/9] mesh: crypto: simplify mesh_crypto_packet_parse() Christian Eggers
@ 2025-07-11 21:57 ` Christian Eggers
  2025-07-15 14:30 ` [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for " patchwork-bot+bluetooth
  9 siblings, 0 replies; 12+ messages in thread
From: Christian Eggers @ 2025-07-11 21:57 UTC (permalink / raw)
  To: Brian Gix, Inga Stotland, linux-bluetooth; +Cc: ceggers

Currently, all relayed packets are broken!

At the point when net_key_encrypt() is called from net_rx() for
encrypting a relay packet, the packet size must include the NetMIC
field. But the length of this field has already been removed during
decryption of the incoming packet (by decrypt_net_pkt()), although
mesh_crypto_packet_decrypt() has correctly reset the NetMIC field to
zeroes.

Move stripping of the NetMIC field length from decrypt_net_pkt() to
mesh_crypto_packet_parse(), so that the field length is only stripped
from the payload message (but keeping the field length for the network
PDU). Additionally add extra length checks during parsing.
---
 mesh/crypto.c   | 21 +++++++++++++++++++++
 mesh/net-keys.c |  5 +----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/mesh/crypto.c b/mesh/crypto.c
index 3dcf226ad8d3..a03dc9483862 100644
--- a/mesh/crypto.c
+++ b/mesh/crypto.c
@@ -637,6 +637,9 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 	if (dst)
 		*dst = this_dst;
 
+	if (packet_len < 9 + 4)
+		return false;
+
 	hdr = l_get_be32(packet + 9);
 
 	is_segmented = !!((hdr >> SEG_HDR_SHIFT) & 0x1);
@@ -646,6 +649,9 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 	if (*ctl) {
 		uint8_t this_opcode = packet[9] & OPCODE_MASK;
 
+		/* NetMIC */
+		packet_len -= 8;
+
 		if (cookie)
 			*cookie = l_get_be32(packet + 2) ^ packet[6];
 
@@ -660,13 +666,22 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 				*seqZero = (hdr >> SEQ_ZERO_HDR_SHIFT) &
 								SEQ_ZERO_MASK;
 
+			if (packet_len < 9)
+				return false;
+
 			*payload = packet + 9;
 			*payload_len = packet_len - 9;
 		} else {
+			if (packet_len < 10)
+				return false;
+
 			*payload = packet + 10;
 			*payload_len = packet_len - 10;
 		}
 	} else {
+		/* NetMIC */
+		packet_len -= 4;
+
 		if (cookie)
 			*cookie = l_get_be32(packet + packet_len - 8);
 
@@ -687,9 +702,15 @@ bool mesh_crypto_packet_parse(const uint8_t *packet, uint8_t packet_len,
 			if (segN)
 				*segN = (hdr >> SEGN_HDR_SHIFT) & SEG_MASK;
 
+			if (packet_len < 13)
+				return false;
+
 			*payload = packet + 13;
 			*payload_len = packet_len - 13;
 		} else {
+			if (packet_len < 10)
+				return false;
+
 			*payload = packet + 10;
 			*payload_len = packet_len - 10;
 		}
diff --git a/mesh/net-keys.c b/mesh/net-keys.c
index 0daeb9209b86..98e6d23d3f87 100644
--- a/mesh/net-keys.c
+++ b/mesh/net-keys.c
@@ -238,10 +238,7 @@ static void decrypt_net_pkt(void *a, void *b)
 
 	if (result) {
 		cache_id = key->id;
-		if (cache_plain[1] & 0x80)
-			cache_plainlen = cache_len - 8;
-		else
-			cache_plainlen = cache_len - 4;
+		cache_plainlen = cache_len;
 	}
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* RE: mesh: cleanups and a bugfix for relay packets
  2025-07-11 21:57 ` [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB Christian Eggers
@ 2025-07-11 23:26   ` bluez.test.bot
  0 siblings, 0 replies; 12+ messages in thread
From: bluez.test.bot @ 2025-07-11 23:26 UTC (permalink / raw)
  To: linux-bluetooth, ceggers

[-- Attachment #1: Type: text/plain, Size: 1497 bytes --]

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=981661

---Test result---

Test Summary:
CheckPatch                    PENDING   0.36 seconds
GitLint                       PENDING   0.35 seconds
BuildEll                      PASS      20.11 seconds
BluezMake                     PASS      2544.29 seconds
MakeCheck                     PASS      20.37 seconds
MakeDistcheck                 PASS      185.20 seconds
CheckValgrind                 PASS      234.67 seconds
CheckSmatch                   WARNING   305.40 seconds
bluezmakeextell               PASS      127.19 seconds
IncrementalBuild              PENDING   0.36 seconds
ScanBuild                     PASS      908.82 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
mesh/mesh-io-mgmt.c:525:67: warning: Variable length array is used.mesh/mesh-io-mgmt.c:525:67: warning: Variable length array is used.
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets
  2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
                   ` (8 preceding siblings ...)
  2025-07-11 21:57 ` [PATCH BlueZ v2 9/9] mesh: fix corrupted relay packets Christian Eggers
@ 2025-07-15 14:30 ` patchwork-bot+bluetooth
  9 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+bluetooth @ 2025-07-15 14:30 UTC (permalink / raw)
  To: Christian Eggers; +Cc: brian.gix, inga.stotland, linux-bluetooth

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 11 Jul 2025 23:57:12 +0200 you wrote:
> The first 3 patches replace further magic numbers by preprocessor
> defines (no functional changes). Patch 4 tries to improve readabilty,
> while 5 and 6 are only cleanups (these patches have already been
> submitted in my previous series, but were not merged without any
> notice).
> 
> Patch 7 and 8 are further simplifications/cleanups, while the last patch
> fixes a serious bug which causes corruption in all relayed packets. Maybe
> this NEVER worked correctly, although this is a major feature of
> Bluetooth Mesh. At leat this one should get reviewied by a Mesh expert.
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=009cfb5188f8
  - [BlueZ,v2,2/9] mesh: introduce MESH_AD_MAX_LEN and MESH_NET_MAX_PDU_LEN
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=646cbe922792
  - [BlueZ,v2,3/9] mesh: replace MESH internal defines by shared ones
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=cd651d8e21d7
  - [BlueZ,v2,4/9] mesh: net: constify tx path
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=b692d72b15b3
  - [BlueZ,v2,5/9] mesh: net: remove unused stuff
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0e586df2d0ae
  - [BlueZ,v2,6/9] mesh: net: update comment
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=cb4b20c71fd3
  - [BlueZ,v2,7/9] mesh: crypto: mesh_crypto_aes_ccm_encrypt(): remove unused parameter
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9a32ff5aa7ed
  - [BlueZ,v2,8/9] mesh: crypto: simplify mesh_crypto_packet_parse()
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=159101c7bc38
  - [BlueZ,v2,9/9] mesh: fix corrupted relay packets
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=2b0a6fa08407

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-15 14:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11 21:57 [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for relay packets Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 1/9] mesh: net-keys: more uses of BEACON_LEN_SNB and BEACON_LEN_MPB Christian Eggers
2025-07-11 23:26   ` mesh: cleanups and a bugfix for relay packets bluez.test.bot
2025-07-11 21:57 ` [PATCH BlueZ v2 2/9] mesh: introduce MESH_AD_MAX_LEN and MESH_NET_MAX_PDU_LEN Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 3/9] mesh: replace MESH internal defines by shared ones Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 4/9] mesh: net: constify tx path Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 5/9] mesh: net: remove unused stuff Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 6/9] mesh: net: update comment Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 7/9] mesh: crypto: mesh_crypto_aes_ccm_encrypt(): remove unused parameter Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 8/9] mesh: crypto: simplify mesh_crypto_packet_parse() Christian Eggers
2025-07-11 21:57 ` [PATCH BlueZ v2 9/9] mesh: fix corrupted relay packets Christian Eggers
2025-07-15 14:30 ` [PATCH Bluez v2 0/9] mesh: cleanups and a bugfix for " patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox