From 50c4465ce4e124af8dc1e94cca741eb72ca539ae Mon Sep 17 00:00:00 2001 From: Inga Stotland Date: Thu, 5 Nov 2020 23:10:53 -0800 Subject: [PATCH] mesh: Fix errors found by static analysis This fixes a NULL pointer dereference error (cfgmod-server.c) and miscellaneous issues related to unchecked return status. Change-Id: Ic0951ce49c17fe1e97065da5f50589563290afdb Signed-off-by: anuj.bhumiya --- 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 b0d523d..5058d0d 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 36728fd..b0db4aa 100644 --- a/mesh/appkey.c +++ b/mesh/appkey.c @@ -99,11 +99,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 2495ddc..af48f9f 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); } @@ -591,12 +590,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 8428ee9..5add8bb 100644 --- a/mesh/keyring.c +++ b/mesh/keyring.c @@ -51,18 +51,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)) + l_debug("failed to mkdir %s", key_file); + 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; } @@ -84,7 +88,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)) + l_debug("failed to mkdir %s", key_file); + snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir, app_idx); l_debug("Put App Key %s", key_file); @@ -101,16 +108,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; } @@ -199,7 +207,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)) + l_debug("failed to mkdir %s", key_file); for (i = 0; i < count; i++) { snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path, -- 2.7.4