[BlueZ] mesh: Fix snprintf return values not being checked

Commit Message

Brian Gix June 29, 2022, 7 p.m. UTC

Some versions of the GCC compiler complain when the return value of
snprintf is not checked. This patch cleans up the Mesh JSON parser.
---
 mesh/mesh-config-json.c | 88 ++++++++++++++++++++++++++---------------
 1 file changed, 57 insertions(+), 31 deletions(-)

Comments

[e-mail] June 29, 2022, 8:15 p.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=655140

---Test result---

Test Summary:
CheckPatch                    FAIL      0.82 seconds
GitLint                       PASS      0.54 seconds
Prep - Setup ELL              PASS      41.91 seconds
Build - Prep                  PASS      0.45 seconds
Build - Configure             PASS      8.07 seconds
Build - Make                  PASS      1222.12 seconds
Make Check                    PASS      11.65 seconds
Make Check w/Valgrind         PASS      441.75 seconds
Make Distcheck                PASS      230.08 seconds
Build w/ext ELL - Configure   PASS      8.10 seconds
Build w/ext ELL - Make        PASS      1197.84 seconds
Incremental Build with patchesPASS      0.00 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ] mesh: Fix snprintf return values not being checked
WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#101: FILE: mesh/mesh-config-json.c:184:
+		return NULL;
+	else

/github/workspace/src/12900623.patch total: 0 errors, 1 warnings, 220 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/12900623.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth

Patch

diff --git a/mesh/mesh-config-json.c b/mesh/mesh-config-json.c
index f142f0e1f..cec636cc3 100644
--- a/mesh/mesh-config-json.c
+++ b/mesh/mesh-config-json.c
@@ -156,7 +156,7 @@  static json_object *get_element_model(json_object *jnode, int ele_idx,
 						uint32_t mod_id, bool vendor)
 {
 	json_object *jelements, *jelement, *jmodels;
-	int i, num_mods;
+	int i, num_mods, ret;
 	size_t len;
 	char buf[9];
 
@@ -174,13 +174,15 @@  static json_object *get_element_model(json_object *jnode, int ele_idx,
 	if (!num_mods)
 		return NULL;
 
-	if (!vendor) {
-		snprintf(buf, 5, "%4.4x", (uint16_t)mod_id);
-		len = 4;
-	} else {
-		snprintf(buf, 9, "%8.8x", mod_id);
-		len = 8;
-	}
+	if (!vendor)
+		ret = snprintf(buf, 5, "%4.4x", (uint16_t)mod_id);
+	else
+		ret = snprintf(buf, 9, "%8.8x", mod_id);
+
+	if (ret < 0)
+		return NULL;
+	else
+		len = ret;
 
 	for (i = 0; i < num_mods; ++i) {
 		json_object *jmodel, *jvalue;
@@ -818,7 +820,7 @@  bool mesh_config_model_binding_add(struct mesh_config *cfg, uint16_t ele_addr,
 							uint16_t app_idx)
 {
 	json_object *jnode, *jmodel, *jstring, *jarray = NULL;
-	int ele_idx;
+	int ele_idx, ret;
 	char buf[5];
 
 	if (!cfg)
@@ -826,6 +828,10 @@  bool mesh_config_model_binding_add(struct mesh_config *cfg, uint16_t ele_addr,
 
 	jnode = cfg->jnode;
 
+	ret = snprintf(buf, 5, "%4.4x", app_idx);
+	if (ret < 0)
+		return false;
+
 	ele_idx = get_element_index(jnode, ele_addr);
 	if (ele_idx < 0)
 		return false;
@@ -834,8 +840,6 @@  bool mesh_config_model_binding_add(struct mesh_config *cfg, uint16_t ele_addr,
 	if (!jmodel)
 		return false;
 
-	snprintf(buf, 5, "%4.4x", app_idx);
-
 	json_object_object_get_ex(jmodel, "bind", &jarray);
 	if (jarray && jarray_has_string(jarray, buf, 4))
 		return true;
@@ -863,7 +867,7 @@  bool mesh_config_model_binding_del(struct mesh_config *cfg, uint16_t ele_addr,
 							uint16_t app_idx)
 {
 	json_object *jnode, *jmodel, *jarray;
-	int ele_idx;
+	int ele_idx, ret;
 	char buf[5];
 
 	if (!cfg)
@@ -871,6 +875,10 @@  bool mesh_config_model_binding_del(struct mesh_config *cfg, uint16_t ele_addr,
 
 	jnode = cfg->jnode;
 
+	ret = snprintf(buf, 5, "%4.4x", app_idx);
+	if (ret < 0)
+		return false;
+
 	ele_idx = get_element_index(jnode, ele_addr);
 	if (ele_idx < 0)
 		return false;
@@ -882,8 +890,6 @@  bool mesh_config_model_binding_del(struct mesh_config *cfg, uint16_t ele_addr,
 	if (!json_object_object_get_ex(jmodel, "bind", &jarray))
 		return true;
 
-	snprintf(buf, 5, "%4.4x", app_idx);
-
 	jarray_string_del(jarray, buf, 4);
 
 	if (!json_object_array_length(jarray))
@@ -1415,9 +1421,13 @@  static bool write_uint16_hex(json_object *jobj, const char *desc,
 								uint16_t value)
 {
 	json_object *jstring;
+	int ret;
 	char buf[5];
 
-	snprintf(buf, 5, "%4.4x", value);
+	ret = snprintf(buf, 5, "%4.4x", value);
+	if (ret < 5)
+		return false;
+
 	jstring = json_object_new_string(buf);
 	if (!jstring)
 		return false;
@@ -1430,9 +1440,13 @@  static bool write_uint16_hex(json_object *jobj, const char *desc,
 static bool write_uint32_hex(json_object *jobj, const char *desc, uint32_t val)
 {
 	json_object *jstring;
+	int ret;
 	char buf[9];
 
-	snprintf(buf, 9, "%8.8x", val);
+	ret = snprintf(buf, 9, "%8.8x", val);
+	if (ret < 0)
+		return false;
+
 	jstring = json_object_new_string(buf);
 	if (!jstring)
 		return false;
@@ -1716,22 +1730,24 @@  struct mesh_config *mesh_config_create(const char *cfgdir_name,
 	char uuid_buf[33];
 	char name_buf[PATH_MAX];
 	struct mesh_config *cfg;
-	size_t max_len = strlen(cfgnode_name) + strlen(bak_ext);
+	int ret;
 
 	if (!hex2str((uint8_t *) uuid, 16, uuid_buf, sizeof(uuid_buf)))
 		return NULL;
 
-	snprintf(name_buf, PATH_MAX, "%s/%s", cfgdir_name, uuid_buf);
-
-	if (strlen(name_buf) + max_len >= PATH_MAX)
+	ret = snprintf(name_buf, PATH_MAX, "%s/%s", cfgdir_name, uuid_buf);
+	if (ret < 0)
 		return NULL;
 
 	/* Create a new directory and node.json file */
 	if (mkdir(name_buf, 0755) != 0)
 		return NULL;
 
-	snprintf(name_buf, PATH_MAX, "%s/%s%s", cfgdir_name, uuid_buf,
+	ret = snprintf(name_buf, PATH_MAX, "%s/%s%s", cfgdir_name, uuid_buf,
 								cfgnode_name);
+	if (ret < 0)
+		return NULL;
+
 	l_debug("New node config %s", name_buf);
 
 	cfg = create_config(name_buf, uuid, db_node);
@@ -1904,12 +1920,14 @@  bool mesh_config_model_pub_del(struct mesh_config *cfg, uint16_t addr,
 static void del_page(json_object *jarray, uint8_t page)
 {
 	char buf[3];
-	int i, len;
+	int i, len, ret;
 
 	if (!jarray)
 		return;
 
-	snprintf(buf, 3, "%2.2x", page);
+	ret = snprintf(buf, 3, "%2.2x", page);
+	if (ret < 0)
+		return;
 
 	len = json_object_array_length(jarray);
 
@@ -1931,7 +1949,7 @@  bool mesh_config_comp_page_add(struct mesh_config *cfg, uint8_t page,
 {
 	json_object *jnode, *jstring, *jarray = NULL;
 	char *buf;
-	int len;
+	int len, ret;
 
 	if (!cfg)
 		return false;
@@ -1942,7 +1960,10 @@  bool mesh_config_comp_page_add(struct mesh_config *cfg, uint8_t page,
 
 	len = (size * 2) + 3;
 	buf = l_malloc(len);
-	snprintf(buf, len, "%2.2x", page);
+	ret = snprintf(buf, len, "%2.2x", page);
+	if (ret < 0)
+		return false;
+
 	hex2str(data, size, buf + 2, len - 2);
 
 	if (jarray && jarray_has_string(jarray, buf, len)) {
@@ -1967,12 +1988,16 @@  bool mesh_config_comp_page_mv(struct mesh_config *cfg, uint8_t old, uint8_t nw)
 	uint8_t *data;
 	char *str;
 	char old_buf[3];
-	int i, len, dlen = 0;
+	int i, len, ret, dlen = 0;
 	bool status = true;
 
 	if (!cfg || old == nw)
 		return false;
 
+	ret = snprintf(old_buf, 3, "%2.2x", old);
+	if (ret < 0)
+		return false;
+
 	jnode = cfg->jnode;
 
 	json_object_object_get_ex(jnode, "pages", &jarray);
@@ -1980,7 +2005,6 @@  bool mesh_config_comp_page_mv(struct mesh_config *cfg, uint8_t old, uint8_t nw)
 	if (!jarray)
 		return false;
 
-	snprintf(old_buf, 3, "%2.2x", old);
 	data = l_malloc(MAX_MSG_LEN);
 
 	len = json_object_array_length(jarray);
@@ -2030,8 +2054,9 @@  bool mesh_config_model_sub_add(struct mesh_config *cfg, uint16_t ele_addr,
 		return false;
 
 	if (!sub->virt) {
-		snprintf(buf, 5, "%4.4x", sub->addr.grp);
-		len = 4;
+		len = snprintf(buf, 5, "%4.4x", sub->addr.grp);
+		if (len < 0)
+			return false;
 	} else {
 		hex2str(sub->addr.label, 16, buf, 33);
 		len = 32;
@@ -2084,8 +2109,9 @@  bool mesh_config_model_sub_del(struct mesh_config *cfg, uint16_t ele_addr,
 		return true;
 
 	if (!sub->virt) {
-		snprintf(buf, 5, "%4.4x", sub->addr.grp);
-		len = 4;
+		len = snprintf(buf, 5, "%4.4x", sub->addr.grp);
+		if (len < 0)
+			return false;
 	} else {
 		hex2str(sub->addr.label, 16, buf, 33);
 		len = 32;