diff mbox series

[BlueZ] mesh: Fix infinite loop on IVIndex update

Message ID 20210226132740.56503-1-przemyslaw.fierek@silvair.com
State New
Headers show
Series [BlueZ] mesh: Fix infinite loop on IVIndex update | expand

Commit Message

Przemysław Fierek Feb. 26, 2021, 1:27 p.m. UTC 
This patch fixes inifinite loop problem caused by recurring call
of the `net_key_beacon_refresh` function.

Problem occurs when at least two nodes are connected to the same
BlueZ instance and they are connected to the same network
(use same network key). Issue is triggered when IVIndex update
process stabilize and one of the nodes receives network beacon
with IVUpdate flag set to 0. Then it processes the "local" beacon
and compose new `snb` (with IVUpdate flag set to 0) attached to
`net_key` instance. After that it calls `net_local_beacon` and
another node processes the new beacon (this node has IVUpdate
flag still set to 1). Note that the `net->ivupdate` has set value 1.
The `update_iv_ivu_state` says that "IVU clear attempted too soon".
The node composes new `snb` with IVUpdate flag set to 1 and writes
it to the `net_key` instance in the `net_key_beacon_refresh`
function. After that it calls `net_local_beacon` which causes
repeat of all process. We are rotating in this loop until end-of-memory.
---
 mesh/net.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

Comments

Brian Gix March 1, 2021, 6:25 p.m. UTC | #1 
Applied, Thanks

On Fri, 2021-02-26 at 14:27 +0100, Przemysław Fierek wrote:
> This patch fixes inifinite loop problem caused by recurring call

> of the `net_key_beacon_refresh` function.

> 

> Problem occurs when at least two nodes are connected to the same

> BlueZ instance and they are connected to the same network

> (use same network key). Issue is triggered when IVIndex update

> process stabilize and one of the nodes receives network beacon

> with IVUpdate flag set to 0. Then it processes the "local" beacon

> and compose new `snb` (with IVUpdate flag set to 0) attached to

> `net_key` instance. After that it calls `net_local_beacon` and

> another node processes the new beacon (this node has IVUpdate

> flag still set to 1). Note that the `net->ivupdate` has set value 1.

> The `update_iv_ivu_state` says that "IVU clear attempted too soon".

> The node composes new `snb` with IVUpdate flag set to 1 and writes

> it to the `net_key` instance in the `net_key_beacon_refresh`

> function. After that it calls `net_local_beacon` which causes

> repeat of all process. We are rotating in this loop until end-of-memory.

> ---

>  mesh/net.c | 34 +++++++++++++++++++++-------------

>  1 file changed, 21 insertions(+), 13 deletions(-)

> 

> diff --git a/mesh/net.c b/mesh/net.c

> index 9624cd058..6acd9bc7b 100644

> --- a/mesh/net.c

> +++ b/mesh/net.c

> @@ -2609,29 +2609,33 @@ static int key_refresh_finish(struct mesh_net *net, uint16_t idx)

>  	return MESH_STATUS_SUCCESS;

>  }

>  

> -static void update_kr_state(struct mesh_subnet *subnet, bool kr, uint32_t id)

> +static bool update_kr_state(struct mesh_subnet *subnet, bool kr, uint32_t id)

>  {

>  	/* Figure out the key refresh phase */

>  	if (kr) {

>  		if (id == subnet->net_key_upd) {

>  			l_debug("Beacon based KR phase 2 change");

> -			key_refresh_phase_two(subnet->net, subnet->idx);

> +			return (key_refresh_phase_two(subnet->net, subnet->idx)

> +							== MESH_STATUS_SUCCESS);

>  		}

>  	} else {

>  		if (id == subnet->net_key_upd) {

>  			l_debug("Beacon based KR phase 3 change");

> -			key_refresh_finish(subnet->net, subnet->idx);

> +			return (key_refresh_finish(subnet->net, subnet->idx)

> +							== MESH_STATUS_SUCCESS);

>  		}

>  	}

> +

> +	return false;

>  }

>  

> -static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

> +static bool update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

>  								bool ivu)

>  {

>  	if ((iv_index - ivu) > (net->iv_index - net->iv_update)) {

>  		/* Don't accept IV_Index changes when performing SAR Out */

>  		if (l_queue_length(net->sar_out))

> -			return;

> +			return false;

>  	}

>  

>  	/* If first beacon seen, accept without judgement */

> @@ -2639,7 +2643,7 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

>  		if (ivu) {

>  			/* Ignore beacons with IVU if IV already updated */

>  			if (iv_index == net->iv_index && !net->iv_update)

> -				return;

> +				return false;

>  

>  			/*

>  			 * Other devices will be accepting old or new iv_index,

> @@ -2660,12 +2664,12 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

>  		if (!net->iv_update &&

>  				net->iv_upd_state == IV_UPD_NORMAL_HOLD) {

>  			l_error("Update attempted too soon");

> -			return;

> +			return false;

>  		}

>  

>  		/* Ignore beacons with IVU if IV already updated */

>  		if (iv_index == net->iv_index)

> -			return;

> +			return false;

>  

>  		if (!net->iv_update) {

>  			l_debug("iv_upd_state = IV_UPD_UPDATING");

> @@ -2675,7 +2679,7 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

>  		}

>  	} else if (net->iv_update) {

>  		l_error("IVU clear attempted too soon");

> -		return;

> +		return false;

>  	}

>  

>  	if ((iv_index - ivu) > (net->iv_index - net->iv_update))

> @@ -2694,10 +2698,12 @@ static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,

>  

>  	net->iv_index = iv_index;

>  	net->iv_update = ivu;

> +	return true;

>  }

>  

>  static void process_beacon(void *net_ptr, void *user_data)

>  {

> +	bool updated = false;

>  	struct mesh_net *net = net_ptr;

>  	struct net_beacon_data *beacon_data = user_data;

>  	uint32_t ivi;

> @@ -2731,13 +2737,15 @@ static void process_beacon(void *net_ptr, void *user_data)

>  	 */

>  	if (net->iv_upd_state == IV_UPD_INIT || ivi != net->iv_index ||

>  							ivu != net->iv_update)

> -		update_iv_ivu_state(net, ivi, ivu);

> +		updated |= update_iv_ivu_state(net, ivi, ivu);

>  

>  	if (kr != local_kr)

> -		update_kr_state(subnet, kr, beacon_data->key_id);

> +		updated |= update_kr_state(subnet, kr, beacon_data->key_id);

>  

> -	net_key_beacon_refresh(beacon_data->key_id, net->iv_index,

> -		!!(subnet->kr_phase == KEY_REFRESH_PHASE_TWO), net->iv_update);

> +	if (updated)

> +		net_key_beacon_refresh(beacon_data->key_id, net->iv_index,

> +				!!(subnet->kr_phase == KEY_REFRESH_PHASE_TWO),

> +								net->iv_update);

>  }

>  

>  static void beacon_recv(void *user_data, struct mesh_io_recv_info *info,
diff mbox series

Patch

diff --git a/mesh/net.c b/mesh/net.c
index 9624cd058..6acd9bc7b 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -2609,29 +2609,33 @@  static int key_refresh_finish(struct mesh_net *net, uint16_t idx)
 	return MESH_STATUS_SUCCESS;
 }
 
-static void update_kr_state(struct mesh_subnet *subnet, bool kr, uint32_t id)
+static bool update_kr_state(struct mesh_subnet *subnet, bool kr, uint32_t id)
 {
 	/* Figure out the key refresh phase */
 	if (kr) {
 		if (id == subnet->net_key_upd) {
 			l_debug("Beacon based KR phase 2 change");
-			key_refresh_phase_two(subnet->net, subnet->idx);
+			return (key_refresh_phase_two(subnet->net, subnet->idx)
+							== MESH_STATUS_SUCCESS);
 		}
 	} else {
 		if (id == subnet->net_key_upd) {
 			l_debug("Beacon based KR phase 3 change");
-			key_refresh_finish(subnet->net, subnet->idx);
+			return (key_refresh_finish(subnet->net, subnet->idx)
+							== MESH_STATUS_SUCCESS);
 		}
 	}
+
+	return false;
 }
 
-static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
+static bool update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
 								bool ivu)
 {
 	if ((iv_index - ivu) > (net->iv_index - net->iv_update)) {
 		/* Don't accept IV_Index changes when performing SAR Out */
 		if (l_queue_length(net->sar_out))
-			return;
+			return false;
 	}
 
 	/* If first beacon seen, accept without judgement */
@@ -2639,7 +2643,7 @@  static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
 		if (ivu) {
 			/* Ignore beacons with IVU if IV already updated */
 			if (iv_index == net->iv_index && !net->iv_update)
-				return;
+				return false;
 
 			/*
 			 * Other devices will be accepting old or new iv_index,
@@ -2660,12 +2664,12 @@  static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
 		if (!net->iv_update &&
 				net->iv_upd_state == IV_UPD_NORMAL_HOLD) {
 			l_error("Update attempted too soon");
-			return;
+			return false;
 		}
 
 		/* Ignore beacons with IVU if IV already updated */
 		if (iv_index == net->iv_index)
-			return;
+			return false;
 
 		if (!net->iv_update) {
 			l_debug("iv_upd_state = IV_UPD_UPDATING");
@@ -2675,7 +2679,7 @@  static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
 		}
 	} else if (net->iv_update) {
 		l_error("IVU clear attempted too soon");
-		return;
+		return false;
 	}
 
 	if ((iv_index - ivu) > (net->iv_index - net->iv_update))
@@ -2694,10 +2698,12 @@  static void update_iv_ivu_state(struct mesh_net *net, uint32_t iv_index,
 
 	net->iv_index = iv_index;
 	net->iv_update = ivu;
+	return true;
 }
 
 static void process_beacon(void *net_ptr, void *user_data)
 {
+	bool updated = false;
 	struct mesh_net *net = net_ptr;
 	struct net_beacon_data *beacon_data = user_data;
 	uint32_t ivi;
@@ -2731,13 +2737,15 @@  static void process_beacon(void *net_ptr, void *user_data)
 	 */
 	if (net->iv_upd_state == IV_UPD_INIT || ivi != net->iv_index ||
 							ivu != net->iv_update)
-		update_iv_ivu_state(net, ivi, ivu);
+		updated |= update_iv_ivu_state(net, ivi, ivu);
 
 	if (kr != local_kr)
-		update_kr_state(subnet, kr, beacon_data->key_id);
+		updated |= update_kr_state(subnet, kr, beacon_data->key_id);
 
-	net_key_beacon_refresh(beacon_data->key_id, net->iv_index,
-		!!(subnet->kr_phase == KEY_REFRESH_PHASE_TWO), net->iv_update);
+	if (updated)
+		net_key_beacon_refresh(beacon_data->key_id, net->iv_index,
+				!!(subnet->kr_phase == KEY_REFRESH_PHASE_TWO),
+								net->iv_update);
 }
 
 static void beacon_recv(void *user_data, struct mesh_io_recv_info *info,