linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ] mesh: Add more checks for element properties
@ 2019-12-11 23:28 Inga Stotland
  2019-12-15 17:59 ` Gix, Brian
  0 siblings, 1 reply; 2+ messages in thread
From: Inga Stotland @ 2019-12-11 23:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, michal.lowas-rzechonek, Inga Stotland

This adds consistency checks for mandatory properties on
org.bluez.mesh.Element1 interface:
    - disallow duplicate models on the same element
    - disallow elements with duplicate indices
    - disallow configuration server model on any element but primary
---
 mesh/node.c | 52 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/mesh/node.c b/mesh/node.c
index f8acc78c3..1f328bd21 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -1081,7 +1081,7 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
 	free_node_dbus_resources(node);
 }
 
-static void get_sig_models_from_properties(struct node_element *ele,
+static bool get_sig_models_from_properties(struct node_element *ele,
 					struct l_dbus_message_iter *property)
 {
 	struct l_dbus_message_iter ids;
@@ -1091,24 +1091,31 @@ static void get_sig_models_from_properties(struct node_element *ele,
 		ele->models = l_queue_new();
 
 	if (!l_dbus_message_iter_get_variant(property, "aq", &ids))
-		return;
+		return false;
 
 	/* Bluetooth SIG defined models */
 	while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
 		struct mesh_model *mod;
 		uint32_t id = mod_id | VENDOR_ID_MASK;
 
+		/* Allow Config Server Model only on the primary element */
+		if (ele->idx != PRIMARY_ELE_IDX && id == CONFIG_SRV_MODEL)
+			return false;
+
+		/* Disallow duplicates */
 		if (l_queue_find(ele->models, match_model_id,
 						L_UINT_TO_PTR(id)))
-			continue;
+			return false;
 
 		mod = mesh_model_new(ele->idx, id);
 
 		l_queue_insert(ele->models, mod, compare_model_id, NULL);
 	}
+
+	return true;
 }
 
-static void get_vendor_models_from_properties(struct node_element *ele,
+static bool get_vendor_models_from_properties(struct node_element *ele,
 					struct l_dbus_message_iter *property)
 {
 	struct l_dbus_message_iter ids;
@@ -1118,21 +1125,24 @@ static void get_vendor_models_from_properties(struct node_element *ele,
 		ele->models = l_queue_new();
 
 	if (!l_dbus_message_iter_get_variant(property, "a(qq)", &ids))
-		return;
+		return false;
 
 	/* Vendor defined models */
 	while (l_dbus_message_iter_next_entry(&ids, &vendor_id, &mod_id)) {
 		struct mesh_model *mod;
 		uint32_t id = mod_id | (vendor_id << 16);
 
+		/* Disallow duplicates */
 		if (l_queue_find(ele->models, match_model_id,
 							L_UINT_TO_PTR(id)))
-			continue;
+			return false;
 
 		mod = mesh_model_new(ele->idx, id);
 
 		l_queue_insert(ele->models, mod, compare_model_id, NULL);
 	}
+
+	return true;
 }
 
 static bool get_element_properties(struct mesh_node *node, const char *path,
@@ -1150,34 +1160,36 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
 	ele->location = DEFAULT_LOCATION;
 
 	while (l_dbus_message_iter_next_entry(properties, &key, &var)) {
-		if (!idx && !strcmp(key, "Index")) {
-			if (!l_dbus_message_iter_get_variant(&var, "y",
+		if (!strcmp(key, "Index")) {
+
+			if (idx || !l_dbus_message_iter_get_variant(&var, "y",
 								&ele->idx))
 				goto fail;
+
 			idx = true;
-			continue;
-		}
 
-		if (!mods && !strcmp(key, "Models")) {
-			get_sig_models_from_properties(ele, &var);
+		} else if (!strcmp(key, "Models")) {
+
+			if (mods || !get_sig_models_from_properties(ele, &var))
+				goto fail;
+
 			mods = true;
-			continue;
-		}
+		} else if (!strcmp(key, "VendorModels")) {
+
+			if (vendor_mods ||
+				!get_vendor_models_from_properties(ele, &var))
+				goto fail;
 
-		if (!vendor_mods && !strcmp(key, "VendorModels")) {
-			get_vendor_models_from_properties(ele, &var);
 			vendor_mods = true;
-			continue;
-		}
 
-		if (!strcmp(key, "Location")) {
+		} else if (!strcmp(key, "Location")) {
 			if (!l_dbus_message_iter_get_variant(&var, "q",
 							&ele->location))
 				goto fail;
-			continue;
 		}
 	}
 
+	/* Check for the presence of the required properties */
 	if (!idx || !mods || !vendor_mods)
 		goto fail;
 
-- 
2.21.0


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

* Re: [PATCH BlueZ] mesh: Add more checks for element properties
  2019-12-11 23:28 [PATCH BlueZ] mesh: Add more checks for element properties Inga Stotland
@ 2019-12-15 17:59 ` Gix, Brian
  0 siblings, 0 replies; 2+ messages in thread
From: Gix, Brian @ 2019-12-15 17:59 UTC (permalink / raw)
  To: linux-bluetooth@vger.kernel.org, Stotland, Inga
  Cc: michal.lowas-rzechonek@silvair.com

Applied

On Wed, 2019-12-11 at 15:28 -0800, Inga Stotland wrote:
> This adds consistency checks for mandatory properties on
> org.bluez.mesh.Element1 interface:
>     - disallow duplicate models on the same element
>     - disallow elements with duplicate indices
>     - disallow configuration server model on any element but primary
> ---
>  mesh/node.c | 52 ++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/mesh/node.c b/mesh/node.c
> index f8acc78c3..1f328bd21 100644
> --- a/mesh/node.c
> +++ b/mesh/node.c
> @@ -1081,7 +1081,7 @@ static void app_disc_cb(struct l_dbus *bus, void *user_data)
>  	free_node_dbus_resources(node);
>  }
>  
> -static void get_sig_models_from_properties(struct node_element *ele,
> +static bool get_sig_models_from_properties(struct node_element *ele,
>  					struct l_dbus_message_iter *property)
>  {
>  	struct l_dbus_message_iter ids;
> @@ -1091,24 +1091,31 @@ static void get_sig_models_from_properties(struct node_element *ele,
>  		ele->models = l_queue_new();
>  
>  	if (!l_dbus_message_iter_get_variant(property, "aq", &ids))
> -		return;
> +		return false;
>  
>  	/* Bluetooth SIG defined models */
>  	while (l_dbus_message_iter_next_entry(&ids, &mod_id)) {
>  		struct mesh_model *mod;
>  		uint32_t id = mod_id | VENDOR_ID_MASK;
>  
> +		/* Allow Config Server Model only on the primary element */
> +		if (ele->idx != PRIMARY_ELE_IDX && id == CONFIG_SRV_MODEL)
> +			return false;
> +
> +		/* Disallow duplicates */
>  		if (l_queue_find(ele->models, match_model_id,
>  						L_UINT_TO_PTR(id)))
> -			continue;
> +			return false;
>  
>  		mod = mesh_model_new(ele->idx, id);
>  
>  		l_queue_insert(ele->models, mod, compare_model_id, NULL);
>  	}
> +
> +	return true;
>  }
>  
> -static void get_vendor_models_from_properties(struct node_element *ele,
> +static bool get_vendor_models_from_properties(struct node_element *ele,
>  					struct l_dbus_message_iter *property)
>  {
>  	struct l_dbus_message_iter ids;
> @@ -1118,21 +1125,24 @@ static void get_vendor_models_from_properties(struct node_element *ele,
>  		ele->models = l_queue_new();
>  
>  	if (!l_dbus_message_iter_get_variant(property, "a(qq)", &ids))
> -		return;
> +		return false;
>  
>  	/* Vendor defined models */
>  	while (l_dbus_message_iter_next_entry(&ids, &vendor_id, &mod_id)) {
>  		struct mesh_model *mod;
>  		uint32_t id = mod_id | (vendor_id << 16);
>  
> +		/* Disallow duplicates */
>  		if (l_queue_find(ele->models, match_model_id,
>  							L_UINT_TO_PTR(id)))
> -			continue;
> +			return false;
>  
>  		mod = mesh_model_new(ele->idx, id);
>  
>  		l_queue_insert(ele->models, mod, compare_model_id, NULL);
>  	}
> +
> +	return true;
>  }
>  
>  static bool get_element_properties(struct mesh_node *node, const char *path,
> @@ -1150,34 +1160,36 @@ static bool get_element_properties(struct mesh_node *node, const char *path,
>  	ele->location = DEFAULT_LOCATION;
>  
>  	while (l_dbus_message_iter_next_entry(properties, &key, &var)) {
> -		if (!idx && !strcmp(key, "Index")) {
> -			if (!l_dbus_message_iter_get_variant(&var, "y",
> +		if (!strcmp(key, "Index")) {
> +
> +			if (idx || !l_dbus_message_iter_get_variant(&var, "y",
>  								&ele->idx))
>  				goto fail;
> +
>  			idx = true;
> -			continue;
> -		}
>  
> -		if (!mods && !strcmp(key, "Models")) {
> -			get_sig_models_from_properties(ele, &var);
> +		} else if (!strcmp(key, "Models")) {
> +
> +			if (mods || !get_sig_models_from_properties(ele, &var))
> +				goto fail;
> +
>  			mods = true;
> -			continue;
> -		}
> +		} else if (!strcmp(key, "VendorModels")) {
> +
> +			if (vendor_mods ||
> +				!get_vendor_models_from_properties(ele, &var))
> +				goto fail;
>  
> -		if (!vendor_mods && !strcmp(key, "VendorModels")) {
> -			get_vendor_models_from_properties(ele, &var);
>  			vendor_mods = true;
> -			continue;
> -		}
>  
> -		if (!strcmp(key, "Location")) {
> +		} else if (!strcmp(key, "Location")) {
>  			if (!l_dbus_message_iter_get_variant(&var, "q",
>  							&ele->location))
>  				goto fail;
> -			continue;
>  		}
>  	}
>  
> +	/* Check for the presence of the required properties */
>  	if (!idx || !mods || !vendor_mods)
>  		goto fail;
>  

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

end of thread, other threads:[~2019-12-15 17:59 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-11 23:28 [PATCH BlueZ] mesh: Add more checks for element properties Inga Stotland
2019-12-15 17:59 ` Gix, Brian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).