diff mbox series

[BlueZ] mesh: Fix errors found by static analysis

Message ID 20201106071053.122820-1-inga.stotland@intel.com
State New
Headers show
Series [BlueZ] mesh: Fix errors found by static analysis | expand

Commit Message

Inga Stotland Nov. 6, 2020, 7:10 a.m. UTC 
This fixes a NULL pointer dereference error (cfgmod-server.c)
and miscellaneous issues related to unchecked return status.
---
 mesh/agent.c         | 43 ++++++++++++++++++++++++++++---------------
 mesh/appkey.c        |  6 +++---
 mesh/cfgmod-server.c | 12 +++---------
 mesh/keyring.c       | 38 ++++++++++++++++++++++++--------------
 4 files changed, 58 insertions(+), 41 deletions(-)

Comments

bluez.test.bot@gmail.com Nov. 6, 2020, 7:26 a.m. UTC | #1 
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=378771

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
Brian Gix Nov. 10, 2020, 6:27 p.m. UTC | #2 
Applied

On Thu, 2020-11-05 at 23:10 -0800, Inga Stotland wrote:
> This fixes a NULL pointer dereference error (cfgmod-server.c)

> and miscellaneous issues related to unchecked return status.

> ---

>  mesh/agent.c         | 43 ++++++++++++++++++++++++++++---------------

>  mesh/appkey.c        |  6 +++---

>  mesh/cfgmod-server.c | 12 +++---------

>  mesh/keyring.c       | 38 ++++++++++++++++++++++++--------------

>  4 files changed, 58 insertions(+), 41 deletions(-)

> 

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

> index b0d523d16..5058d0d8d 100644

> --- a/mesh/agent.c

> +++ b/mesh/agent.c

> @@ -95,7 +95,7 @@ static bool simple_match(const void *a, const void *b)

>  	return a == b;

>  }

>  

> -static void parse_prov_caps(struct mesh_agent_prov_caps *caps,

> +static bool parse_prov_caps(struct mesh_agent_prov_caps *caps,

>  				struct l_dbus_message_iter *property)

>  {

>  	struct l_dbus_message_iter iter_caps;

> @@ -103,7 +103,7 @@ static void parse_prov_caps(struct mesh_agent_prov_caps *caps,

>  	uint32_t i;

>  

>  	if (!l_dbus_message_iter_get_variant(property, "as", &iter_caps))

> -		return;

> +		return false;

>  

>  	while (l_dbus_message_iter_next_entry(&iter_caps, &str)) {

>  		for (i = 0; i < L_ARRAY_SIZE(cap_table); i++) {

> @@ -129,9 +129,10 @@ static void parse_prov_caps(struct mesh_agent_prov_caps *caps,

>  			caps->static_type = 1;

>  	}

>  

> +	return true;

>  }

>  

> -static void parse_oob_info(struct mesh_agent_prov_caps *caps,

> +static bool parse_oob_info(struct mesh_agent_prov_caps *caps,

>  				struct l_dbus_message_iter *property)

>  {

>  	struct l_dbus_message_iter iter_oob;

> @@ -139,7 +140,7 @@ static void parse_oob_info(struct mesh_agent_prov_caps *caps,

>  	const char *str;

>  

>  	if (!l_dbus_message_iter_get_variant(property, "as", &iter_oob))

> -		return;

> +		return false;

>  

>  	while (l_dbus_message_iter_next_entry(&iter_oob, &str)) {

>  		for (i = 0; i < L_ARRAY_SIZE(oob_table); i++) {

> @@ -148,9 +149,11 @@ static void parse_oob_info(struct mesh_agent_prov_caps *caps,

>  			caps->oob_info |= oob_table[i].mask;

>  		}

>  	}

> +

> +	return true;

>  }

>  

> -static void parse_properties(struct mesh_agent *agent,

> +static bool parse_properties(struct mesh_agent *agent,

>  					struct l_dbus_message_iter *properties)

>  {

>  	const char *key, *uri_string;

> @@ -160,15 +163,20 @@ static void parse_properties(struct mesh_agent *agent,

>  

>  	while (l_dbus_message_iter_next_entry(properties, &key, &variant)) {

>  		if (!strcmp(key, "Capabilities")) {

> -			parse_prov_caps(&agent->caps, &variant);

> +			if (!parse_prov_caps(&agent->caps, &variant))

> +				return false;

>  		} else if (!strcmp(key, "URI")) {

> -			l_dbus_message_iter_get_variant(&variant, "s",

> -								&uri_string);

> +			if (!l_dbus_message_iter_get_variant(&variant, "s",

> +								&uri_string))

> +				return false;

>  			/* TODO: compute hash */

>  		} else if (!strcmp(key, "OutOfBandInfo")) {

> -			parse_oob_info(&agent->caps, &variant);

> +			if (!parse_oob_info(&agent->caps, &variant))

> +				return false;

>  		}

>  	}

> +

> +	return true;

>  }

>  

>  static void agent_free(void *agent_data)

> @@ -253,7 +261,10 @@ struct mesh_agent *mesh_agent_create(const char *path, const char *owner,

>  	agent->owner = l_strdup(owner);

>  	agent->path = l_strdup(path);

>  

> -	parse_properties(agent, properties);

> +	if (!parse_properties(agent, properties)) {

> +		l_free(agent);

> +		return NULL;

> +	}

>  

>  	l_queue_push_tail(agents, agent);

>  

> @@ -312,15 +323,17 @@ static void properties_reply(struct l_dbus_message *reply, void *user_data)

>  	err = get_reply_error(reply);

>  

>  	if (err != MESH_ERROR_NONE)

> -		goto fail;

> +		goto done;

>  

>  	if (!l_dbus_message_get_arguments(reply, "a{sv}", &properties)) {

>  		err = MESH_ERROR_FAILED;

> -		goto fail;

> +		goto done;

>  	}

>  

> -	parse_properties(agent, &properties);

> -fail:

> +	if (!parse_properties(agent, &properties))

> +		err = MESH_ERROR_FAILED;

> +

> +done:

>  	if (req->cb) {

>  		cb = req->cb;

>  		cb(req->user_data, err);

> @@ -425,7 +438,7 @@ static void key_reply(struct l_dbus_message *reply, void *user_data)

>  	mesh_agent_key_cb_t cb;

>  	struct l_dbus_message_iter iter_array;

>  	uint32_t n = 0, expected_len = 0;

> -	uint8_t *buf;

> +	uint8_t *buf = NULL;

>  	int err;

>  

>  	if (!l_queue_find(agents, simple_match, agent) || !agent->req)

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

> index 7efb8f5c4..549f5a80d 100644

> --- a/mesh/appkey.c

> +++ b/mesh/appkey.c

> @@ -100,11 +100,11 @@ bool appkey_key_init(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,

>  	if (!app_keys)

>  		return NULL;

>  

> -	key = app_key_new();

> -	if (!key)

> +	if (!mesh_net_have_key(net, net_idx))

>  		return false;

>  

> -	if (!mesh_net_have_key(net, net_idx))

> +	key = app_key_new();

> +	if (!key)

>  		return false;

>  

>  	key->net_idx = net_idx;

> diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c

> index 0009a1bee..5eefedc4d 100644

> --- a/mesh/cfgmod-server.c

> +++ b/mesh/cfgmod-server.c

> @@ -80,13 +80,12 @@ static uint16_t config_pub_get(struct mesh_node *node, const uint8_t *pkt,

>  

>  	pub = mesh_model_pub_get(node, ele_addr, id, &status);

>  

> -	rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) - 1) << 3);

> -

> -	if (pub && status == MESH_STATUS_SUCCESS)

> +	if (pub && status == MESH_STATUS_SUCCESS) {

> +		rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) - 1) << 3);

>  		return set_pub_status(status, ele_addr, id, pub->addr, pub->idx,

>  					pub->credential, pub->ttl, pub->period,

>  					rtx);

> -	else

> +	} else

>  		return set_pub_status(status, ele_addr, id, 0, 0, 0, 0, 0, 0);

>  }

>  

> @@ -592,12 +591,7 @@ static uint16_t cfg_appkey_msg(struct mesh_node *node, const uint8_t *pkt,

>  	struct mesh_net *net = node_get_net(node);

>  

>  	n_idx = l_get_le16(pkt) & 0xfff;

> -	if (n_idx > NET_IDX_MAX)

> -		return 0;

> -

>  	a_idx = l_get_le16(pkt + 1) >> 4;

> -	if (a_idx > APP_IDX_MAX)

> -		return 0;

>  

>  	n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);

>  

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

> index bbe501063..1ef4fc3ef 100644

> --- a/mesh/keyring.c

> +++ b/mesh/keyring.c

> @@ -50,18 +50,22 @@ bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,

>  		return false;

>  

>  	snprintf(key_file, PATH_MAX, "%s%s", node_path, net_key_dir);

> -	mkdir(key_file, 0755);

> +

> +	if (!mkdir(key_file, 0755))

> +		return false;

> +

>  	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,

>  								net_idx);

>  	l_debug("Put Net Key %s", key_file);

>  

>  	fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);

> -	if (fd >= 0) {

> -		if (write(fd, key, sizeof(*key)) == sizeof(*key))

> -			result = true;

> +	if (fd < 0)

> +		return false;

>  

> -		close(fd);

> -	}

> +	if (write(fd, key, sizeof(*key)) == sizeof(*key))

> +		result = true;

> +

> +	close(fd);

>  

>  	return result;

>  }

> @@ -83,7 +87,10 @@ bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,

>  		return false;

>  

>  	snprintf(key_file, PATH_MAX, "%s%s", node_path, app_key_dir);

> -	mkdir(key_file, 0755);

> +

> +	if (!mkdir(key_file, 0755))

> +		return false;

> +

>  	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,

>  								app_idx);

>  	l_debug("Put App Key %s", key_file);

> @@ -100,16 +107,17 @@ bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,

>  		}

>  

>  		lseek(fd, 0, SEEK_SET);

> -	} else

> +	} else {

>  		fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,

>  							S_IRUSR | S_IWUSR);

> +		if (fd < 0)

> +			return false;

> +	}

>  

> -	if (fd >= 0) {

> -		if (write(fd, key, sizeof(*key)) == sizeof(*key))

> -			result = true;

> +	if (write(fd, key, sizeof(*key)) == sizeof(*key))

> +		result = true;

>  

> -		close(fd);

> -	}

> +	close(fd);

>  

>  	return result;

>  }

> @@ -198,7 +206,9 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,

>  		return false;

>  

>  	snprintf(key_file, PATH_MAX, "%s%s", node_path, dev_key_dir);

> -	mkdir(key_file, 0755);

> +

> +	if (!mkdir(key_file, 0755))

> +		return false;

>  

>  	for (i = 0; i < count; i++) {

>  		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
Steve Brown Nov. 28, 2020, 5:05 p.m. UTC | #3 
Hi Inga,

The changes to mesh/keyring.c break create network for me.

I think the mkdir tests should be 
if (!!mkdir(key_file, 0755))
not 
if (!mkdir(key_file, 0755))

With that change, I can create networks again.

Steve


On Tue, 2020-11-10 at 18:27 +0000, Gix, Brian wrote:
> Applied

> 

> On Thu, 2020-11-05 at 23:10 -0800, Inga Stotland wrote:

> > This fixes a NULL pointer dereference error (cfgmod-server.c)

> > and miscellaneous issues related to unchecked return status.

> > ---

> >  mesh/agent.c         | 43 ++++++++++++++++++++++++++++------------

> > ---

> >  mesh/appkey.c        |  6 +++---

> >  mesh/cfgmod-server.c | 12 +++---------

> >  mesh/keyring.c       | 38 ++++++++++++++++++++++++--------------

> >  4 files changed, 58 insertions(+), 41 deletions(-)

> > 

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

> > index b0d523d16..5058d0d8d 100644

> > --- a/mesh/agent.c

> > +++ b/mesh/agent.c

> > @@ -95,7 +95,7 @@ static bool simple_match(const void *a, const

> > void *b)

> >         return a == b;

> >  }

> >  

> > -static void parse_prov_caps(struct mesh_agent_prov_caps *caps,

> > +static bool parse_prov_caps(struct mesh_agent_prov_caps *caps,

> >                                 struct l_dbus_message_iter

> > *property)

> >  {

> >         struct l_dbus_message_iter iter_caps;

> > @@ -103,7 +103,7 @@ static void parse_prov_caps(struct

> > mesh_agent_prov_caps *caps,

> >         uint32_t i;

> >  

> >         if (!l_dbus_message_iter_get_variant(property, "as",

> > &iter_caps))

> > -               return;

> > +               return false;

> >  

> >         while (l_dbus_message_iter_next_entry(&iter_caps, &str)) {

> >                 for (i = 0; i < L_ARRAY_SIZE(cap_table); i++) {

> > @@ -129,9 +129,10 @@ static void parse_prov_caps(struct

> > mesh_agent_prov_caps *caps,

> >                         caps->static_type = 1;

> >         }

> >  

> > +       return true;

> >  }

> >  

> > -static void parse_oob_info(struct mesh_agent_prov_caps *caps,

> > +static bool parse_oob_info(struct mesh_agent_prov_caps *caps,

> >                                 struct l_dbus_message_iter

> > *property)

> >  {

> >         struct l_dbus_message_iter iter_oob;

> > @@ -139,7 +140,7 @@ static void parse_oob_info(struct

> > mesh_agent_prov_caps *caps,

> >         const char *str;

> >  

> >         if (!l_dbus_message_iter_get_variant(property, "as",

> > &iter_oob))

> > -               return;

> > +               return false;

> >  

> >         while (l_dbus_message_iter_next_entry(&iter_oob, &str)) {

> >                 for (i = 0; i < L_ARRAY_SIZE(oob_table); i++) {

> > @@ -148,9 +149,11 @@ static void parse_oob_info(struct

> > mesh_agent_prov_caps *caps,

> >                         caps->oob_info |= oob_table[i].mask;

> >                 }

> >         }

> > +

> > +       return true;

> >  }

> >  

> > -static void parse_properties(struct mesh_agent *agent,

> > +static bool parse_properties(struct mesh_agent *agent,

> >                                         struct l_dbus_message_iter

> > *properties)

> >  {

> >         const char *key, *uri_string;

> > @@ -160,15 +163,20 @@ static void parse_properties(struct

> > mesh_agent *agent,

> >  

> >         while (l_dbus_message_iter_next_entry(properties, &key,

> > &variant)) {

> >                 if (!strcmp(key, "Capabilities")) {

> > -                       parse_prov_caps(&agent->caps, &variant);

> > +                       if (!parse_prov_caps(&agent->caps,

> > &variant))

> > +                               return false;

> >                 } else if (!strcmp(key, "URI")) {

> > -                       l_dbus_message_iter_get_variant(&variant,

> > "s",

> > -

> >                                                                &uri_

> > string);

> > +                       if

> > (!l_dbus_message_iter_get_variant(&variant, "s",

> > +                                                               &ur

> > i_string))

> > +                               return false;

> >                         /* TODO: compute hash */

> >                 } else if (!strcmp(key, "OutOfBandInfo")) {

> > -                       parse_oob_info(&agent->caps, &variant);

> > +                       if (!parse_oob_info(&agent->caps,

> > &variant))

> > +                               return false;

> >                 }

> >         }

> > +

> > +       return true;

> >  }

> >  

> >  static void agent_free(void *agent_data)

> > @@ -253,7 +261,10 @@ struct mesh_agent *mesh_agent_create(const

> > char *path, const char *owner,

> >         agent->owner = l_strdup(owner);

> >         agent->path = l_strdup(path);

> >  

> > -       parse_properties(agent, properties);

> > +       if (!parse_properties(agent, properties)) {

> > +               l_free(agent);

> > +               return NULL;

> > +       }

> >  

> >         l_queue_push_tail(agents, agent);

> >  

> > @@ -312,15 +323,17 @@ static void properties_reply(struct

> > l_dbus_message *reply, void *user_data)

> >         err = get_reply_error(reply);

> >  

> >         if (err != MESH_ERROR_NONE)

> > -               goto fail;

> > +               goto done;

> >  

> >         if (!l_dbus_message_get_arguments(reply, "a{sv}",

> > &properties)) {

> >                 err = MESH_ERROR_FAILED;

> > -               goto fail;

> > +               goto done;

> >         }

> >  

> > -       parse_properties(agent, &properties);

> > -fail:

> > +       if (!parse_properties(agent, &properties))

> > +               err = MESH_ERROR_FAILED;

> > +

> > +done:

> >         if (req->cb) {

> >                 cb = req->cb;

> >                 cb(req->user_data, err);

> > @@ -425,7 +438,7 @@ static void key_reply(struct l_dbus_message

> > *reply, void *user_data)

> >         mesh_agent_key_cb_t cb;

> >         struct l_dbus_message_iter iter_array;

> >         uint32_t n = 0, expected_len = 0;

> > -       uint8_t *buf;

> > +       uint8_t *buf = NULL;

> >         int err;

> >  

> >         if (!l_queue_find(agents, simple_match, agent) || !agent-

> > >req)

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

> > index 7efb8f5c4..549f5a80d 100644

> > --- a/mesh/appkey.c

> > +++ b/mesh/appkey.c

> > @@ -100,11 +100,11 @@ bool appkey_key_init(struct mesh_net *net,

> > uint16_t net_idx, uint16_t app_idx,

> >         if (!app_keys)

> >                 return NULL;

> >  

> > -       key = app_key_new();

> > -       if (!key)

> > +       if (!mesh_net_have_key(net, net_idx))

> >                 return false;

> >  

> > -       if (!mesh_net_have_key(net, net_idx))

> > +       key = app_key_new();

> > +       if (!key)

> >                 return false;

> >  

> >         key->net_idx = net_idx;

> > diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c

> > index 0009a1bee..5eefedc4d 100644

> > --- a/mesh/cfgmod-server.c

> > +++ b/mesh/cfgmod-server.c

> > @@ -80,13 +80,12 @@ static uint16_t config_pub_get(struct mesh_node

> > *node, const uint8_t *pkt,

> >  

> >         pub = mesh_model_pub_get(node, ele_addr, id, &status);

> >  

> > -       rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) - 1) << 3);

> > -

> > -       if (pub && status == MESH_STATUS_SUCCESS)

> > +       if (pub && status == MESH_STATUS_SUCCESS) {

> > +               rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) -

> > 1) << 3);

> >                 return set_pub_status(status, ele_addr, id, pub-

> > >addr, pub->idx,

> >                                         pub->credential, pub->ttl,

> > pub->period,

> >                                         rtx);

> > -       else

> > +       } else

> >                 return set_pub_status(status, ele_addr, id, 0, 0,

> > 0, 0, 0, 0);

> >  }

> >  

> > @@ -592,12 +591,7 @@ static uint16_t cfg_appkey_msg(struct

> > mesh_node *node, const uint8_t *pkt,

> >         struct mesh_net *net = node_get_net(node);

> >  

> >         n_idx = l_get_le16(pkt) & 0xfff;

> > -       if (n_idx > NET_IDX_MAX)

> > -               return 0;

> > -

> >         a_idx = l_get_le16(pkt + 1) >> 4;

> > -       if (a_idx > APP_IDX_MAX)

> > -               return 0;

> >  

> >         n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);

> >  

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

> > index bbe501063..1ef4fc3ef 100644

> > --- a/mesh/keyring.c

> > +++ b/mesh/keyring.c

> > @@ -50,18 +50,22 @@ bool keyring_put_net_key(struct mesh_node

> > *node, uint16_t net_idx,

> >                 return false;

> >  

> >         snprintf(key_file, PATH_MAX, "%s%s", node_path,

> > net_key_dir);

> > -       mkdir(key_file, 0755);

> > +

> > +       if (!mkdir(key_file, 0755))

> > +               return false;

> > +

> >         snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path,

> > net_key_dir,

> >                                                                 net

> > _idx);

> >         l_debug("Put Net Key %s", key_file);

> >  

> >         fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR |

> > S_IWUSR);

> > -       if (fd >= 0) {

> > -               if (write(fd, key, sizeof(*key)) == sizeof(*key))

> > -                       result = true;

> > +       if (fd < 0)

> > +               return false;

> >  

> > -               close(fd);

> > -       }

> > +       if (write(fd, key, sizeof(*key)) == sizeof(*key))

> > +               result = true;

> > +

> > +       close(fd);

> >  

> >         return result;

> >  }

> > @@ -83,7 +87,10 @@ bool keyring_put_app_key(struct mesh_node *node,

> > uint16_t app_idx,

> >                 return false;

> >  

> >         snprintf(key_file, PATH_MAX, "%s%s", node_path,

> > app_key_dir);

> > -       mkdir(key_file, 0755);

> > +

> > +       if (!mkdir(key_file, 0755))

> > +               return false;

> > +

> >         snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path,

> > app_key_dir,

> >                                                                 app

> > _idx);

> >         l_debug("Put App Key %s", key_file);

> > @@ -100,16 +107,17 @@ bool keyring_put_app_key(struct mesh_node

> > *node, uint16_t app_idx,

> >                 }

> >  

> >                 lseek(fd, 0, SEEK_SET);

> > -       } else

> > +       } else {

> >                 fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,

> >                                                         S_IRUSR |

> > S_IWUSR);

> > +               if (fd < 0)

> > +                       return false;

> > +       }

> >  

> > -       if (fd >= 0) {

> > -               if (write(fd, key, sizeof(*key)) == sizeof(*key))

> > -                       result = true;

> > +       if (write(fd, key, sizeof(*key)) == sizeof(*key))

> > +               result = true;

> >  

> > -               close(fd);

> > -       }

> > +       close(fd);

> >  

> >         return result;

> >  }

> > @@ -198,7 +206,9 @@ bool keyring_put_remote_dev_key(struct

> > mesh_node *node, uint16_t unicast,

> >                 return false;

> >  

> >         snprintf(key_file, PATH_MAX, "%s%s", node_path,

> > dev_key_dir);

> > -       mkdir(key_file, 0755);

> > +

> > +       if (!mkdir(key_file, 0755))

> > +               return false;

> >  

> >         for (i = 0; i < count; i++) {

> >                 snprintf(key_file, PATH_MAX, "%s%s/%4.4x",

> > node_path,
Inga Stotland Nov. 29, 2020, 5:10 a.m. UTC | #4 
Hi Steve,

On Sat, 2020-11-28 at 12:05 -0500, Steve Brown wrote:
> Hi Inga,

> 

> The changes to mesh/keyring.c break create network for me.

> 

> I think the mkdir tests should be 

> if (!!mkdir(key_file, 0755))


Yes, you are correct: it's a wrong check. Will fix.

> not 

> if (!mkdir(key_file, 0755))

> 

> With that change, I can create networks again.

> 

> Steve

> 


Best regards,
Inga
diff mbox series

Patch

diff --git a/mesh/agent.c b/mesh/agent.c
index b0d523d16..5058d0d8d 100644
--- a/mesh/agent.c
+++ b/mesh/agent.c
@@ -95,7 +95,7 @@  static bool simple_match(const void *a, const void *b)
 	return a == b;
 }
 
-static void parse_prov_caps(struct mesh_agent_prov_caps *caps,
+static bool parse_prov_caps(struct mesh_agent_prov_caps *caps,
 				struct l_dbus_message_iter *property)
 {
 	struct l_dbus_message_iter iter_caps;
@@ -103,7 +103,7 @@  static void parse_prov_caps(struct mesh_agent_prov_caps *caps,
 	uint32_t i;
 
 	if (!l_dbus_message_iter_get_variant(property, "as", &iter_caps))
-		return;
+		return false;
 
 	while (l_dbus_message_iter_next_entry(&iter_caps, &str)) {
 		for (i = 0; i < L_ARRAY_SIZE(cap_table); i++) {
@@ -129,9 +129,10 @@  static void parse_prov_caps(struct mesh_agent_prov_caps *caps,
 			caps->static_type = 1;
 	}
 
+	return true;
 }
 
-static void parse_oob_info(struct mesh_agent_prov_caps *caps,
+static bool parse_oob_info(struct mesh_agent_prov_caps *caps,
 				struct l_dbus_message_iter *property)
 {
 	struct l_dbus_message_iter iter_oob;
@@ -139,7 +140,7 @@  static void parse_oob_info(struct mesh_agent_prov_caps *caps,
 	const char *str;
 
 	if (!l_dbus_message_iter_get_variant(property, "as", &iter_oob))
-		return;
+		return false;
 
 	while (l_dbus_message_iter_next_entry(&iter_oob, &str)) {
 		for (i = 0; i < L_ARRAY_SIZE(oob_table); i++) {
@@ -148,9 +149,11 @@  static void parse_oob_info(struct mesh_agent_prov_caps *caps,
 			caps->oob_info |= oob_table[i].mask;
 		}
 	}
+
+	return true;
 }
 
-static void parse_properties(struct mesh_agent *agent,
+static bool parse_properties(struct mesh_agent *agent,
 					struct l_dbus_message_iter *properties)
 {
 	const char *key, *uri_string;
@@ -160,15 +163,20 @@  static void parse_properties(struct mesh_agent *agent,
 
 	while (l_dbus_message_iter_next_entry(properties, &key, &variant)) {
 		if (!strcmp(key, "Capabilities")) {
-			parse_prov_caps(&agent->caps, &variant);
+			if (!parse_prov_caps(&agent->caps, &variant))
+				return false;
 		} else if (!strcmp(key, "URI")) {
-			l_dbus_message_iter_get_variant(&variant, "s",
-								&uri_string);
+			if (!l_dbus_message_iter_get_variant(&variant, "s",
+								&uri_string))
+				return false;
 			/* TODO: compute hash */
 		} else if (!strcmp(key, "OutOfBandInfo")) {
-			parse_oob_info(&agent->caps, &variant);
+			if (!parse_oob_info(&agent->caps, &variant))
+				return false;
 		}
 	}
+
+	return true;
 }
 
 static void agent_free(void *agent_data)
@@ -253,7 +261,10 @@  struct mesh_agent *mesh_agent_create(const char *path, const char *owner,
 	agent->owner = l_strdup(owner);
 	agent->path = l_strdup(path);
 
-	parse_properties(agent, properties);
+	if (!parse_properties(agent, properties)) {
+		l_free(agent);
+		return NULL;
+	}
 
 	l_queue_push_tail(agents, agent);
 
@@ -312,15 +323,17 @@  static void properties_reply(struct l_dbus_message *reply, void *user_data)
 	err = get_reply_error(reply);
 
 	if (err != MESH_ERROR_NONE)
-		goto fail;
+		goto done;
 
 	if (!l_dbus_message_get_arguments(reply, "a{sv}", &properties)) {
 		err = MESH_ERROR_FAILED;
-		goto fail;
+		goto done;
 	}
 
-	parse_properties(agent, &properties);
-fail:
+	if (!parse_properties(agent, &properties))
+		err = MESH_ERROR_FAILED;
+
+done:
 	if (req->cb) {
 		cb = req->cb;
 		cb(req->user_data, err);
@@ -425,7 +438,7 @@  static void key_reply(struct l_dbus_message *reply, void *user_data)
 	mesh_agent_key_cb_t cb;
 	struct l_dbus_message_iter iter_array;
 	uint32_t n = 0, expected_len = 0;
-	uint8_t *buf;
+	uint8_t *buf = NULL;
 	int err;
 
 	if (!l_queue_find(agents, simple_match, agent) || !agent->req)
diff --git a/mesh/appkey.c b/mesh/appkey.c
index 7efb8f5c4..549f5a80d 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -100,11 +100,11 @@  bool appkey_key_init(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
 	if (!app_keys)
 		return NULL;
 
-	key = app_key_new();
-	if (!key)
+	if (!mesh_net_have_key(net, net_idx))
 		return false;
 
-	if (!mesh_net_have_key(net, net_idx))
+	key = app_key_new();
+	if (!key)
 		return false;
 
 	key->net_idx = net_idx;
diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 0009a1bee..5eefedc4d 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -80,13 +80,12 @@  static uint16_t config_pub_get(struct mesh_node *node, const uint8_t *pkt,
 
 	pub = mesh_model_pub_get(node, ele_addr, id, &status);
 
-	rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) - 1) << 3);
-
-	if (pub && status == MESH_STATUS_SUCCESS)
+	if (pub && status == MESH_STATUS_SUCCESS) {
+		rtx = pub->rtx.cnt + (((pub->rtx.interval / 50) - 1) << 3);
 		return set_pub_status(status, ele_addr, id, pub->addr, pub->idx,
 					pub->credential, pub->ttl, pub->period,
 					rtx);
-	else
+	} else
 		return set_pub_status(status, ele_addr, id, 0, 0, 0, 0, 0, 0);
 }
 
@@ -592,12 +591,7 @@  static uint16_t cfg_appkey_msg(struct mesh_node *node, const uint8_t *pkt,
 	struct mesh_net *net = node_get_net(node);
 
 	n_idx = l_get_le16(pkt) & 0xfff;
-	if (n_idx > NET_IDX_MAX)
-		return 0;
-
 	a_idx = l_get_le16(pkt + 1) >> 4;
-	if (a_idx > APP_IDX_MAX)
-		return 0;
 
 	n = mesh_model_opcode_set(OP_APPKEY_STATUS, msg);
 
diff --git a/mesh/keyring.c b/mesh/keyring.c
index bbe501063..1ef4fc3ef 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -50,18 +50,22 @@  bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
 		return false;
 
 	snprintf(key_file, PATH_MAX, "%s%s", node_path, net_key_dir);
-	mkdir(key_file, 0755);
+
+	if (!mkdir(key_file, 0755))
+		return false;
+
 	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
 								net_idx);
 	l_debug("Put Net Key %s", key_file);
 
 	fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
-	if (fd >= 0) {
-		if (write(fd, key, sizeof(*key)) == sizeof(*key))
-			result = true;
+	if (fd < 0)
+		return false;
 
-		close(fd);
-	}
+	if (write(fd, key, sizeof(*key)) == sizeof(*key))
+		result = true;
+
+	close(fd);
 
 	return result;
 }
@@ -83,7 +87,10 @@  bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
 		return false;
 
 	snprintf(key_file, PATH_MAX, "%s%s", node_path, app_key_dir);
-	mkdir(key_file, 0755);
+
+	if (!mkdir(key_file, 0755))
+		return false;
+
 	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
 								app_idx);
 	l_debug("Put App Key %s", key_file);
@@ -100,16 +107,17 @@  bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
 		}
 
 		lseek(fd, 0, SEEK_SET);
-	} else
+	} else {
 		fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
 							S_IRUSR | S_IWUSR);
+		if (fd < 0)
+			return false;
+	}
 
-	if (fd >= 0) {
-		if (write(fd, key, sizeof(*key)) == sizeof(*key))
-			result = true;
+	if (write(fd, key, sizeof(*key)) == sizeof(*key))
+		result = true;
 
-		close(fd);
-	}
+	close(fd);
 
 	return result;
 }
@@ -198,7 +206,9 @@  bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
 		return false;
 
 	snprintf(key_file, PATH_MAX, "%s%s", node_path, dev_key_dir);
-	mkdir(key_file, 0755);
+
+	if (!mkdir(key_file, 0755))
+		return false;
 
 	for (i = 0; i < count; i++) {
 		snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,