Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Tedd Ho-Jeong An <hj.tedd.an@gmail.com>
To:     linux-bluetooth@vger.kernel.org
Subject: [BlueZ PATCH 1/9] device: Fix unchecked return value
Date:   Mon, 18 Oct 2021 10:28:25 -0700
Message-Id: <20211018172833.534191-2-hj.tedd.an@gmail.com>
In-Reply-To: <20211018172833.534191-1-hj.tedd.an@gmail.com>
References: <20211018172833.534191-1-hj.tedd.an@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Tedd Ho-Jeong An <tedd.an@intel.com>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
 src/device.c | 141 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 119 insertions(+), 22 deletions(-)

diff --git a/src/device.c b/src/device.c
index ac83cdc9c..d5aae9576 100644
--- a/src/device.c
+++ b/src/device.c
@@ -379,6 +379,7 @@ static gboolean store_device_info_cb(gpointer user_data)
 {
 	struct btd_device *device = user_data;
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char filename[PATH_MAX];
 	char device_addr[18];
 	char *str;
@@ -394,7 +395,11 @@ static gboolean store_device_info_cb(gpointer user_data)
 				device_addr);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	g_key_file_set_string(key_file, "General", "Name", device->name);
 
@@ -467,7 +472,12 @@ static gboolean store_device_info_cb(gpointer user_data)
 	create_file(filename, 0600);
 
 	str = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(filename, str, length, NULL);
+	if (!g_file_set_contents(filename, str, length, &gerr)) {
+		error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
+
 	g_free(str);
 
 	g_key_file_free(key_file);
@@ -509,6 +519,7 @@ void device_store_cached_name(struct btd_device *dev, const char *name)
 	char filename[PATH_MAX];
 	char d_addr[18];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char *data;
 	char *data_old;
 	gsize length = 0;
@@ -526,16 +537,25 @@ void device_store_cached_name(struct btd_device *dev, const char *name)
 	create_file(filename, 0600);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
+
 	data_old = g_key_file_to_data(key_file, &length_old, NULL);
 
 	g_key_file_set_string(key_file, "General", "Name", name);
 
 	data = g_key_file_to_data(key_file, &length, NULL);
 
-	if ((length != length_old) || (memcmp(data, data_old, length)))
-		g_file_set_contents(filename, data, length, NULL);
-
+	if ((length != length_old) || (memcmp(data, data_old, length))) {
+		if (!g_file_set_contents(filename, data, length, &gerr)) {
+			error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+			g_error_free(gerr);
+		}
+	}
 	g_free(data);
 	g_free(data_old);
 
@@ -2306,6 +2326,7 @@ static void store_services(struct btd_device *device)
 	uuid_t uuid;
 	char *prim_uuid;
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	GSList *l;
 	char *data;
 	gsize length = 0;
@@ -2363,7 +2384,11 @@ static void store_services(struct btd_device *device)
 	data = g_key_file_to_data(key_file, &length, NULL);
 	if (length > 0) {
 		create_file(filename, 0600);
-		g_file_set_contents(filename, data, length, NULL);
+		if (!g_file_set_contents(filename, data, length, &gerr)) {
+			error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+			g_error_free(gerr);
+		}
 	}
 
 	free(prim_uuid);
@@ -2532,6 +2557,7 @@ static void store_gatt_db(struct btd_device *device)
 	char filename[PATH_MAX];
 	char dst_addr[18];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char *data;
 	gsize length = 0;
 	struct gatt_saver saver;
@@ -2553,7 +2579,11 @@ static void store_gatt_db(struct btd_device *device)
 	create_file(filename, 0600);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	/* Remove current attributes since it might have changed */
 	g_key_file_remove_group(key_file, "Attributes", NULL);
@@ -2564,7 +2594,11 @@ static void store_gatt_db(struct btd_device *device)
 	gatt_db_foreach_service(device->db, NULL, store_service, &saver);
 
 	data = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(filename, data, length, NULL);
+	if (!g_file_set_contents(filename, data, length, &gerr)) {
+		error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	g_free(data);
 	g_key_file_free(key_file);
@@ -3295,6 +3329,7 @@ static void convert_info(struct btd_device *device, GKeyFile *key_file)
 	char **uuids;
 	char *str;
 	gsize length = 0;
+	GError *gerr = NULL;
 
 	/* Load device profile list from legacy properties */
 	uuids = g_key_file_get_string_list(key_file, "General", "SDPServices",
@@ -3320,7 +3355,11 @@ static void convert_info(struct btd_device *device, GKeyFile *key_file)
 			device_addr);
 
 	str = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(filename, str, length, NULL);
+	if (!g_file_set_contents(filename, str, length, &gerr)) {
+		error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	g_free(str);
 
 	store_device_info(device);
@@ -3466,6 +3505,7 @@ static void load_att_info(struct btd_device *device, const char *local,
 {
 	char filename[PATH_MAX];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char *prim_uuid, *str;
 	char **groups, **handle, *service_uuid;
 	struct gatt_primary *prim;
@@ -3480,7 +3520,11 @@ static void load_att_info(struct btd_device *device, const char *local,
 			peer);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	groups = g_key_file_get_groups(key_file, NULL);
 
 	for (handle = groups; *handle; handle++) {
@@ -3856,6 +3900,7 @@ static void load_gatt_db(struct btd_device *device, const char *local,
 {
 	char **keys, filename[PATH_MAX];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 
 	if (!gatt_cache_is_enabled(device))
 		return;
@@ -3865,7 +3910,11 @@ static void load_gatt_db(struct btd_device *device, const char *local,
 	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", local, peer);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	keys = g_key_file_get_keys(key_file, "Attributes", NULL, NULL);
 
 	if (!keys) {
@@ -4516,6 +4565,7 @@ static void device_remove_stored(struct btd_device *device)
 	char device_addr[18];
 	char filename[PATH_MAX];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char *data;
 	gsize length = 0;
 
@@ -4543,14 +4593,22 @@ static void device_remove_stored(struct btd_device *device)
 				device_addr);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	g_key_file_remove_group(key_file, "ServiceRecords", NULL);
 	g_key_file_remove_group(key_file, "Attributes", NULL);
 
 	data = g_key_file_to_data(key_file, &length, NULL);
 	if (length > 0) {
 		create_file(filename, 0600);
-		g_file_set_contents(filename, data, length, NULL);
+		if (!g_file_set_contents(filename, data, length, &gerr)) {
+			error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+			g_error_free(gerr);
+		}
 	}
 
 	g_free(data);
@@ -4929,6 +4987,8 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs)
 	char att_file[PATH_MAX];
 	GKeyFile *sdp_key_file;
 	GKeyFile *att_key_file;
+	GError *gerr = NULL;
+
 	char *data;
 	gsize length = 0;
 
@@ -4939,13 +4999,21 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs)
 								dstaddr);
 
 	sdp_key_file = g_key_file_new();
-	g_key_file_load_from_file(sdp_key_file, sdp_file, 0, NULL);
+	if (!g_key_file_load_from_file(sdp_key_file, sdp_file, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", sdp_file,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	snprintf(att_file, PATH_MAX, STORAGEDIR "/%s/%s/attributes", srcaddr,
 								dstaddr);
 
 	att_key_file = g_key_file_new();
-	g_key_file_load_from_file(att_key_file, att_file, 0, NULL);
+	if (!g_key_file_load_from_file(att_key_file, att_file, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", att_file,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	for (seq = recs; seq; seq = seq->next) {
 		sdp_record_t *rec = (sdp_record_t *) seq->data;
@@ -5000,7 +5068,12 @@ next:
 		data = g_key_file_to_data(sdp_key_file, &length, NULL);
 		if (length > 0) {
 			create_file(sdp_file, 0600);
-			g_file_set_contents(sdp_file, data, length, NULL);
+			if (!g_file_set_contents(sdp_file, data, length,
+								&gerr)) {
+				error("Unable set contents for %s: (%s)",
+						sdp_file, gerr->message);
+				g_error_free(gerr);
+			}
 		}
 
 		g_free(data);
@@ -5011,7 +5084,12 @@ next:
 		data = g_key_file_to_data(att_key_file, &length, NULL);
 		if (length > 0) {
 			create_file(att_file, 0600);
-			g_file_set_contents(att_file, data, length, NULL);
+			if (!g_file_set_contents(att_file, data, length,
+								&gerr)) {
+				error("Unable set contents for %s: (%s)",
+						att_file, gerr->message);
+				g_error_free(gerr);
+			}
 		}
 
 		g_free(data);
@@ -5897,6 +5975,7 @@ void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
 	char filename[PATH_MAX];
 	char device_addr[18];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	uint16_t old_value;
 	gsize length = 0;
 	char *str;
@@ -5907,7 +5986,11 @@ void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
 				device_addr);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	/* for bonded devices this is done on every connection so limit writes
 	 * to storage if no change needed
@@ -5933,7 +6016,11 @@ void device_store_svc_chng_ccc(struct btd_device *device, uint8_t bdaddr_type,
 	create_file(filename, 0600);
 
 	str = g_key_file_to_data(key_file, &length, NULL);
-	g_file_set_contents(filename, str, length, NULL);
+	if (!g_file_set_contents(filename, str, length, &gerr)) {
+		error("Unable set contents for %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	g_free(str);
 
 done:
@@ -5945,6 +6032,7 @@ void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
 	char filename[PATH_MAX];
 	char device_addr[18];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 
 	ba2str(&device->bdaddr, device_addr);
 	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
@@ -5952,7 +6040,11 @@ void device_load_svc_chng_ccc(struct btd_device *device, uint16_t *ccc_le,
 				device_addr);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 
 	if (!g_key_file_has_group(key_file, "ServiceChanged")) {
 		if (ccc_le)
@@ -6794,6 +6886,7 @@ static sdp_list_t *read_device_records(struct btd_device *device)
 	char local[18], peer[18];
 	char filename[PATH_MAX];
 	GKeyFile *key_file;
+	GError *gerr = NULL;
 	char **keys, **handle;
 	char *str;
 	sdp_list_t *recs = NULL;
@@ -6805,7 +6898,11 @@ static sdp_list_t *read_device_records(struct btd_device *device)
 	snprintf(filename, PATH_MAX, STORAGEDIR "/%s/cache/%s", local, peer);
 
 	key_file = g_key_file_new();
-	g_key_file_load_from_file(key_file, filename, 0, NULL);
+	if (!g_key_file_load_from_file(key_file, filename, 0, &gerr)) {
+		error("Unable to load key file from %s: (%s)", filename,
+								gerr->message);
+		g_error_free(gerr);
+	}
 	keys = g_key_file_get_keys(key_file, "ServiceRecords", NULL, NULL);
 
 	for (handle = keys; handle && *handle; handle++) {
-- 
2.25.1