[PATCH BlueZ 0/2] Fix AppKey deletion

[PATCH BlueZ 0/2] Fix AppKey deletion

[Inga Stotland]

This patch set contains fixes for:
 - deleting individual app key deletion as a result receiving
 Config AppKey Delete message. The fix removes a dulpicate attempt
 to write to config storage and some function call simplifications

- deleting a number of appkeys as a result of receiving
Config NetKey Delete message. When deleting multiple entries from a
queue, do not use queue iteration mechanism as the deletion of
multiple entries may result in seg fault. Instead, find and delete
bound keys one by one until none are found.
 

Inga Stotland (2):
  mesh: Fix logic in AppKey deletion
  mesh: Fix wholesale deletion of appkeys bound to a netkey

 mesh/appkey.c | 32 ++++++++++++++++++++++++--------
 mesh/node.c   | 29 ++---------------------------
 mesh/node.h   |  4 ++--
 3 files changed, 28 insertions(+), 37 deletions(-)

-- 
2.21.1

[PATCH BlueZ 1/2] mesh: Fix logic in AppKey deletion

[Inga Stotland]

When deleting an AppKey from a node, call mesh_config_app_key_del()
only from appkey level (i.e., keep it contained in appkey.c and
remove the duplicate call in node.c)

Also, simplify the node_app_key_delete() argument list.
---
 mesh/appkey.c |  6 +++---
 mesh/node.c   | 29 ++---------------------------
 mesh/node.h   |  4 ++--
 3 files changed, 7 insertions(+), 32 deletions(-)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index e96d5228a..3a1fd8a54 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -419,13 +419,13 @@ int appkey_key_delete(struct mesh_net *net, uint16_t net_idx,
 	if (key->net_idx != net_idx)
 		return MESH_STATUS_INVALID_NETKEY;
 
-	node_app_key_delete(net, mesh_net_get_address(net), net_idx, app_idx);
+	node = mesh_net_node_get(net);
+
+	node_app_key_delete(node, net_idx, app_idx);
 
 	l_queue_remove(app_keys, key);
 	appkey_key_free(key);
 
-	node = mesh_net_node_get(net);
-
 	if (!mesh_config_app_key_del(node_config_get(node), net_idx, app_idx))
 		return MESH_STATUS_STORAGE_FAIL;
 
diff --git a/mesh/node.c b/mesh/node.c
index 3154d6bf4..78219d9d7 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -135,15 +135,6 @@ struct managed_obj_request {
 
 static struct l_queue *nodes;
 
-static bool match_node_unicast(const void *a, const void *b)
-{
-	const struct mesh_node *node = a;
-	uint16_t dst = L_PTR_TO_UINT(b);
-
-	return (dst >= node->primary &&
-		dst <= (node->primary + node->num_ele - 1));
-}
-
 static bool match_device_uuid(const void *a, const void *b)
 {
 	const struct mesh_node *node = a;
@@ -216,15 +207,6 @@ static int compare_model_id(const void *a, const void *b, void *user_data)
 	return 0;
 }
 
-
-struct mesh_node *node_find_by_addr(uint16_t addr)
-{
-	if (!IS_UNICAST(addr))
-		return NULL;
-
-	return l_queue_find(nodes, match_node_unicast, L_UINT_TO_PTR(addr));
-}
-
 struct mesh_node *node_find_by_uuid(uint8_t uuid[16])
 {
 	return l_queue_find(nodes, match_device_uuid, uuid);
@@ -627,24 +609,17 @@ bool node_is_provisioned(struct mesh_node *node)
 	return (!IS_UNASSIGNED(node->primary));
 }
 
-bool node_app_key_delete(struct mesh_net *net, uint16_t addr,
-				uint16_t net_idx, uint16_t app_idx)
+void node_app_key_delete(struct mesh_node *node, uint16_t net_idx,
+							uint16_t app_idx)
 {
-	struct mesh_node *node;
 	const struct l_queue_entry *entry;
 
-	node = node_find_by_addr(addr);
-	if (!node)
-		return false;
-
 	entry = l_queue_get_entries(node->elements);
 	for (; entry; entry = entry->next) {
 		struct node_element *ele = entry->data;
 
 		mesh_model_app_key_delete(node, ele->models, app_idx);
 	}
-
-	return mesh_config_app_key_del(node->cfg, net_idx, app_idx);
 }
 
 uint16_t node_get_primary(struct mesh_node *node)
diff --git a/mesh/node.h b/mesh/node.h
index c718c038d..a6bc4a2a6 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -40,8 +40,8 @@ struct mesh_node *node_find_by_uuid(uint8_t uuid[16]);
 struct mesh_node *node_find_by_token(uint64_t token);
 bool node_is_provisioner(struct mesh_node *node);
 bool node_is_provisioned(struct mesh_node *node);
-bool node_app_key_delete(struct mesh_net *net, uint16_t addr,
-				uint16_t net_idx, uint16_t idx);
+void node_app_key_delete(struct mesh_node *node, uint16_t net_idx,
+							uint16_t app_idx);
 uint16_t node_get_primary(struct mesh_node *node);
 uint16_t node_get_primary_net_idx(struct mesh_node *node);
 void node_set_token(struct mesh_node *node, uint8_t token[8]);
-- 
2.21.1

[PATCH BlueZ 2/2] mesh: Fix wholesale deletion of appkeys bound to a netkey

[Inga Stotland]

When a netkey is deleted all the appkeys bound to this key has
to be deleted as well. This fixes app_key queue manipulation to
avoid issues caused by modifying the queue while iterating over it:
instead of iteration over all the entries, find a first bound key,
delete it, find next... and so on, until there are no bound keys
left in the app_keys queue.
---
 mesh/appkey.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index 3a1fd8a54..0eb268782 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -58,6 +58,14 @@ static bool match_key_index(const void *a, const void *b)
 	return key->app_idx == idx;
 }
 
+static bool match_bound_key(const void *a, const void *b)
+{
+	const struct mesh_app_key *key = a;
+	uint16_t idx = L_PTR_TO_UINT(b);
+
+	return key->net_idx == idx;
+}
+
 static bool match_replay_cache(const void *a, const void *b)
 {
 	const struct mesh_msg *msg = a;
@@ -434,19 +442,27 @@ int appkey_key_delete(struct mesh_net *net, uint16_t net_idx,
 
 void appkey_delete_bound_keys(struct mesh_net *net, uint16_t net_idx)
 {
-	const struct l_queue_entry *entry;
 	struct l_queue *app_keys;
+	struct mesh_node *node;
+	struct mesh_app_key *key;
 
 	app_keys = mesh_net_get_app_keys(net);
 	if (!app_keys)
 		return;
 
-	entry = l_queue_get_entries(app_keys);
+	node = mesh_net_node_get(net);
 
-	for (; entry; entry = entry->next) {
-		struct mesh_app_key *key = entry->data;
+	key = l_queue_remove_if(app_keys, match_bound_key,
+					L_UINT_TO_PTR(net_idx));
+
+	while (key) {
+		node_app_key_delete(node, net_idx, key->app_idx);
+		mesh_config_app_key_del(node_config_get(node), net_idx,
+								key->app_idx);
+		appkey_key_free(key);
 
-		appkey_key_delete(net, net_idx, key->app_idx);
+		key = l_queue_remove_if(app_keys, match_bound_key,
+					L_UINT_TO_PTR(net_idx));
 	}
 }
 
-- 
2.21.1

Re: [PATCH BlueZ 0/2] Fix AppKey deletion

[Gix, Brian]

Patchset Applied
On Thu, 2020-01-09 at 17:41 -0800, Inga Stotland wrote:
> This patch set contains fixes for:
>  - deleting individual app key deletion as a result receiving
>  Config AppKey Delete message. The fix removes a dulpicate attempt
>  to write to config storage and some function call simplifications
> 
> - deleting a number of appkeys as a result of receiving
> Config NetKey Delete message. When deleting multiple entries from a
> queue, do not use queue iteration mechanism as the deletion of
> multiple entries may result in seg fault. Instead, find and delete
> bound keys one by one until none are found.
>  
> 
> Inga Stotland (2):
>   mesh: Fix logic in AppKey deletion
>   mesh: Fix wholesale deletion of appkeys bound to a netkey
> 
>  mesh/appkey.c | 32 ++++++++++++++++++++++++--------
>  mesh/node.c   | 29 ++---------------------------
>  mesh/node.h   |  4 ++--
>  3 files changed, 28 insertions(+), 37 deletions(-)
>