2021-10-18 17:30:50

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 0/9] Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch series fixes the unchecked return value(CWE-252) issues
reported by the Coverity scan.

The idea is to make the change as simple as possible without changing
the functional flow. So, it logs the output when the function fails
after checking the return value.

Tedd Ho-Jeong An (9):
device: Fix unchecked return value
adapter: Fix unchecked return value
attrib-server: Fix unchecked return value
plugins/admin: Fix unchecked return value
profiles/a2dp: Fix unchecked return value
profiles/input: Fix unchecked return value
mesh: Fix unchecked return value
obexd: Fix unchecked return value
peripheral: Fix unchecked return value

mesh/keyring.c | 6 +-
mesh/mesh-io-unit.c | 6 +-
mesh/rpl.c | 22 ++--
mesh/util.c | 11 +-
obexd/client/transfer.c | 12 +-
obexd/plugins/pcsuite.c | 4 +-
obexd/src/main.c | 4 +-
peripheral/main.c | 3 +-
plugins/admin.c | 15 ++-
profiles/audio/a2dp.c | 33 +++++-
profiles/input/device.c | 6 +-
src/adapter.c | 249 ++++++++++++++++++++++++++++++++++------
src/attrib-server.c | 21 +++-
src/device.c | 141 +++++++++++++++++++----
14 files changed, 441 insertions(+), 92 deletions(-)

--
2.25.1


2021-10-18 17:30:52

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 8/9] obexd: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
obexd/client/transfer.c | 12 +++++++++---
obexd/plugins/pcsuite.c | 4 +++-
obexd/src/main.c | 4 +++-
3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
index 744d8f106..dccce03b4 100644
--- a/obexd/client/transfer.c
+++ b/obexd/client/transfer.c
@@ -420,8 +420,11 @@ static void obc_transfer_free(struct obc_transfer *transfer)

if (transfer->op == G_OBEX_OP_GET &&
transfer->status != TRANSFER_STATUS_COMPLETE &&
- transfer->filename)
- remove(transfer->filename);
+ transfer->filename) {
+ if (remove(transfer->filename) < 0)
+ error("remove(%s): %s(%d)", transfer->filename,
+ strerror(errno), errno);
+ }

if (transfer->fd > 0)
close(transfer->fd);
@@ -521,7 +524,10 @@ static gboolean transfer_open(struct obc_transfer *transfer, int flags,
}

if (transfer->filename == NULL) {
- remove(filename); /* remove always only if NULL was given */
+ /* remove always only if NULL was given */
+ if (remove(filename) < 0)
+ error("remove(%s): %s(%d)", filename, strerror(errno),
+ errno);
g_free(filename);
} else {
g_free(transfer->filename);
diff --git a/obexd/plugins/pcsuite.c b/obexd/plugins/pcsuite.c
index b2232ea09..f5a9d9ae8 100644
--- a/obexd/plugins/pcsuite.c
+++ b/obexd/plugins/pcsuite.c
@@ -219,7 +219,9 @@ static void pcsuite_disconnect(struct obex_session *os, void *user_data)
close(pcsuite->fd);

if (pcsuite->lock_file) {
- remove(pcsuite->lock_file);
+ if (remove(pcsuite->lock_file) < 0)
+ error("remove(%s): %s(%d)", pcsuite->lock_file,
+ strerror(errno), errno);
g_free(pcsuite->lock_file);
}

diff --git a/obexd/src/main.c b/obexd/src/main.c
index 04284c9e1..d950883f0 100644
--- a/obexd/src/main.c
+++ b/obexd/src/main.c
@@ -270,7 +270,9 @@ int main(int argc, char *argv[])
if (option_root == NULL) {
option_root = g_build_filename(g_get_user_cache_dir(), "obexd",
NULL);
- g_mkdir_with_parents(option_root, 0700);
+ if (g_mkdir_with_parents(option_root, 0700) < 0)
+ error("Failed to create dir(%d): %s", errno,
+ option_root);
}

if (option_root[0] != '/') {
--
2.25.1

2021-10-18 17:30:52

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 2/9] adapter: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
src/adapter.c | 249 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 211 insertions(+), 38 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5846f0396..54b6322cc 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -515,6 +515,7 @@ uint8_t btd_adapter_get_address_type(struct btd_adapter *adapter)
static void store_adapter_info(struct btd_adapter *adapter)
{
GKeyFile *key_file;
+ GError *gerr = NULL;
char filename[PATH_MAX];
char *str;
gsize length = 0;
@@ -550,7 +551,11 @@ static void store_adapter_info(struct btd_adapter *adapter)
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);
@@ -3933,6 +3938,7 @@ static int generate_and_write_irk(uint8_t *irk, GKeyFile *key_file,
struct bt_crypto *crypto;
char str_irk_out[33];
gsize length = 0;
+ GError *gerr = NULL;
char *str;
int i;

@@ -3959,7 +3965,11 @@ static int generate_and_write_irk(uint8_t *irk, GKeyFile *key_file,
g_key_file_set_string(key_file, "General", "IdentityResolvingKey",
str_irk_out);
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);
DBG("Generated IRK written to file");
return 0;
@@ -3969,6 +3979,7 @@ static int load_irk(struct btd_adapter *adapter, uint8_t *irk)
{
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *str_irk;
int ret;

@@ -3976,7 +3987,11 @@ static int load_irk(struct btd_adapter *adapter, uint8_t *irk)
btd_adapter_get_storage_dir(adapter));

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);
+ }

str_irk = g_key_file_get_string(key_file, "General",
"IdentityResolvingKey", NULL);
@@ -4664,6 +4679,7 @@ static void load_devices(struct btd_adapter *adapter)
GSList *irks = NULL;
GSList *params = NULL;
GSList *added_devices = NULL;
+ GError *gerr = NULL;
DIR *dir;
struct dirent *entry;

@@ -4701,7 +4717,11 @@ static void load_devices(struct btd_adapter *adapter)
entry->d_name);

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);
+ }

key_info = get_key_info(key_file, entry->d_name);

@@ -5686,6 +5706,7 @@ static void convert_names_entry(char *key, char *value, void *user_data)
char *str = key;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *data;
gsize length = 0;

@@ -5699,11 +5720,19 @@ static void convert_names_entry(char *key, char *value, void *user_data)
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);
+ }
g_key_file_set_string(key_file, "General", "Name", value);

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);
@@ -5897,6 +5926,7 @@ static void convert_entry(char *key, char *value, void *user_data)
char type = BDADDR_BREDR;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *data;
gsize length = 0;

@@ -5924,7 +5954,11 @@ static void convert_entry(char *key, char *value, void *user_data)
converter->address, key);

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);
+ }

set_device_type(key_file, type);

@@ -5933,7 +5967,11 @@ static void convert_entry(char *key, char *value, void *user_data)
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);
@@ -6014,6 +6052,7 @@ static void store_sdp_record(char *local, char *peer, int handle, char *value)
{
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char handle_str[11];
char *data;
gsize length = 0;
@@ -6021,7 +6060,11 @@ static void store_sdp_record(char *local, char *peer, int handle, char *value)
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);
+ }

sprintf(handle_str, "0x%8.8X", handle);
g_key_file_set_string(key_file, "ServiceRecords", handle_str, value);
@@ -6029,7 +6072,11 @@ static void store_sdp_record(char *local, char *peer, int handle, char *value)
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);
@@ -6045,6 +6092,7 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
int handle, ret;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
struct stat st;
sdp_record_t *rec;
uuid_t uuid;
@@ -6096,14 +6144,22 @@ static void convert_sdp_entry(char *key, char *value, void *user_data)
dst_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);
+ }

store_attribute_uuid(key_file, start, end, prim_uuid, uuid);

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);
@@ -6123,6 +6179,7 @@ static void convert_primaries_entry(char *key, char *value, void *user_data)
char **services, **service, *prim_uuid;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
int ret;
uint16_t start, end;
char uuid_str[MAX_LEN_UUID_STR + 1];
@@ -6147,7 +6204,11 @@ static void convert_primaries_entry(char *key, char *value, void *user_data)
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/attributes", address,
key);
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 (service = services; *service; service++) {
ret = sscanf(*service, "%04hX#%04hX#%s", &start, &end,
@@ -6168,7 +6229,11 @@ static void convert_primaries_entry(char *key, char *value, void *user_data)
goto end;

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);
+ }

if (device_type < 0)
goto end;
@@ -6179,13 +6244,21 @@ static void convert_primaries_entry(char *key, char *value, void *user_data)
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info", address, key);

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);
+ }
set_device_type(key_file, device_type);

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);
+ }
}

end:
@@ -6203,6 +6276,7 @@ static void convert_ccc_entry(char *key, char *value, void *user_data)
int ret, err;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
struct stat st;
char group[6];
char *data;
@@ -6226,7 +6300,11 @@ static void convert_ccc_entry(char *key, char *value, void *user_data)
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/ccc", src_addr,
dst_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);
+ }

sprintf(group, "%hu", handle);
g_key_file_set_string(key_file, group, "Value", value);
@@ -6234,7 +6312,11 @@ static void convert_ccc_entry(char *key, char *value, void *user_data)
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);
@@ -6250,6 +6332,7 @@ static void convert_gatt_entry(char *key, char *value, void *user_data)
int ret, err;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
struct stat st;
char group[6];
char *data;
@@ -6273,7 +6356,11 @@ static void convert_gatt_entry(char *key, char *value, void *user_data)
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/gatt", src_addr,
dst_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);
+ }

sprintf(group, "%hu", handle);
g_key_file_set_string(key_file, group, "Value", value);
@@ -6281,7 +6368,11 @@ static void convert_gatt_entry(char *key, char *value, void *user_data)
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);
@@ -6294,6 +6385,7 @@ static void convert_proximity_entry(char *key, char *value, void *user_data)
char *alert;
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
struct stat st;
int err;
char *data;
@@ -6319,14 +6411,22 @@ static void convert_proximity_entry(char *key, char *value, void *user_data)
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/proximity", src_addr,
key);
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, alert, "Level", value);

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);
@@ -6402,6 +6502,7 @@ static void convert_config(struct btd_adapter *adapter, const char *filename,
uint8_t mode;
char *data;
gsize length = 0;
+ GError *gerr = NULL;

ba2str(&adapter->bdaddr, address);
snprintf(config_path, PATH_MAX, STORAGEDIR "/%s/config", address);
@@ -6426,7 +6527,11 @@ static void convert_config(struct btd_adapter *adapter, const char *filename,
create_file(filename, 0600);

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);
}

@@ -6510,7 +6615,11 @@ static void load_config(struct btd_adapter *adapter)
convert_device_storage(adapter);
}

- 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);
+ }

/* Get alias */
adapter->stored_alias = g_key_file_get_string(key_file, "General",
@@ -8222,6 +8331,7 @@ static void store_link_key(struct btd_adapter *adapter,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
gsize length = 0;
char key_str[33];
char *str;
@@ -8232,7 +8342,11 @@ static void store_link_key(struct btd_adapter *adapter,
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
btd_adapter_get_storage_dir(adapter), 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 (i = 0; i < 16; i++)
sprintf(key_str + (i * 2), "%2.2X", key[i]);
@@ -8245,7 +8359,11 @@ static void store_link_key(struct btd_adapter *adapter,
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);
@@ -8307,6 +8425,7 @@ static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char key_str[33];
gsize length = 0;
char *str;
@@ -8322,7 +8441,11 @@ static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
btd_adapter_get_storage_dir(adapter), 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);
+ }

/* Old files may contain this so remove it in case it exists */
g_key_file_remove_key(key_file, "LongTermKey", "Master", NULL);
@@ -8342,7 +8465,11 @@ static void store_longtermkey(struct btd_adapter *adapter, const bdaddr_t *peer,
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);
@@ -8420,6 +8547,7 @@ static void store_csrk(struct btd_adapter *adapter, const bdaddr_t *peer,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char key_str[33];
gsize length = 0;
gboolean auth;
@@ -8454,7 +8582,11 @@ static void store_csrk(struct btd_adapter *adapter, const bdaddr_t *peer,
btd_adapter_get_storage_dir(adapter), 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 (i = 0; i < 16; i++)
sprintf(key_str + (i * 2), "%2.2X", key[i]);
@@ -8466,7 +8598,11 @@ static void store_csrk(struct btd_adapter *adapter, const bdaddr_t *peer,
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);
@@ -8514,6 +8650,7 @@ static void store_irk(struct btd_adapter *adapter, const bdaddr_t *peer,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *store_data;
char str[33];
size_t length = 0;
@@ -8524,7 +8661,11 @@ static void store_irk(struct btd_adapter *adapter, const bdaddr_t *peer,
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
btd_adapter_get_storage_dir(adapter), 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 (i = 0; i < 16; i++)
sprintf(str + (i * 2), "%2.2X", key[i]);
@@ -8534,7 +8675,11 @@ static void store_irk(struct btd_adapter *adapter, const bdaddr_t *peer,
create_file(filename, 0600);

store_data = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(filename, store_data, length, NULL);
+ if (!g_file_set_contents(filename, store_data, length, &gerr)) {
+ error("Unable set contents for %s: (%s)", filename,
+ gerr->message);
+ g_error_free(gerr);
+ }
g_free(store_data);

g_key_file_free(key_file);
@@ -8602,6 +8747,7 @@ static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *store_data;
size_t length = 0;

@@ -8612,7 +8758,11 @@ static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
btd_adapter_get_storage_dir(adapter), 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_integer(key_file, "ConnectionParameters",
"MinInterval", min_interval);
@@ -8626,7 +8776,11 @@ static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
create_file(filename, 0600);

store_data = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(filename, store_data, length, NULL);
+ if (!g_file_set_contents(filename, store_data, length, &gerr)) {
+ error("Unable set contents for %s: (%s)", filename,
+ gerr->message);
+ g_error_free(gerr);
+ }
g_free(store_data);

g_key_file_free(key_file);
@@ -9254,6 +9408,7 @@ static void remove_keys(struct btd_adapter *adapter,
char device_addr[18];
char filename[PATH_MAX];
GKeyFile *key_file;
+ GError *gerr = NULL;
gsize length = 0;
char *str;

@@ -9262,7 +9417,11 @@ static void remove_keys(struct btd_adapter *adapter,
snprintf(filename, PATH_MAX, STORAGEDIR "/%s/%s/info",
btd_adapter_get_storage_dir(adapter), 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 (type == BDADDR_BREDR) {
g_key_file_remove_group(key_file, "LinkKey", NULL);
@@ -9274,7 +9433,11 @@ static void remove_keys(struct btd_adapter *adapter,
}

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);
@@ -9345,6 +9508,7 @@ static bool get_static_addr(struct btd_adapter *adapter)
{
struct bt_crypto *crypto;
GKeyFile *file;
+ GError *gerr = NULL;
char **addrs;
char mfg[7];
char *str;
@@ -9354,7 +9518,12 @@ static bool get_static_addr(struct btd_adapter *adapter)
snprintf(mfg, sizeof(mfg), "0x%04x", adapter->manufacturer);

file = g_key_file_new();
- g_key_file_load_from_file(file, STORAGEDIR "/addresses", 0, NULL);
+ if (!g_key_file_load_from_file(file, STORAGEDIR "/addresses", 0,
+ &gerr)) {
+ error("Unable to load key file from %s: (%s)",
+ STORAGEDIR "/addresses", gerr->message);
+ g_error_free(gerr);
+ }
addrs = g_key_file_get_string_list(file, "Static", mfg, &len, NULL);
if (addrs) {
for (i = 0; i < len; i++) {
@@ -9408,7 +9577,11 @@ static bool get_static_addr(struct btd_adapter *adapter)
(const char **)addrs, len);

str = g_key_file_to_data(file, &len, NULL);
- g_file_set_contents(STORAGEDIR "/addresses", str, len, NULL);
+ if (!g_file_set_contents(STORAGEDIR "/addresses", str, len, &gerr)) {
+ error("Unable set contents for %s: (%s)",
+ STORAGEDIR "/addresses", gerr->message);
+ g_error_free(gerr);
+ }
g_free(str);

ret = true;
--
2.25.1

2021-10-18 17:31:04

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 5/9] profiles/a2dp: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
profiles/audio/a2dp.c | 33 ++++++++++++++++++++++++++++-----
1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 031ece628..eba2f9822 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -816,6 +816,7 @@ static void store_remote_seps(struct a2dp_channel *chan)
char filename[PATH_MAX];
char dst_addr[18];
GKeyFile *key_file;
+ GError *gerr = NULL;
char *data;
gsize length = 0;

@@ -828,7 +829,11 @@ static void store_remote_seps(struct a2dp_channel *chan)
btd_adapter_get_storage_dir(device_get_adapter(device)),
dst_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);
+ }

data = g_key_file_get_string(key_file, "Endpoints", "LastUsed",
NULL);
@@ -845,7 +850,11 @@ static void store_remote_seps(struct a2dp_channel *chan)
}

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);
@@ -981,6 +990,7 @@ static void store_last_used(struct a2dp_channel *chan, uint8_t lseid,
uint8_t rseid)
{
GKeyFile *key_file;
+ GError *gerr = NULL;
char filename[PATH_MAX];
char dst_addr[18];
char value[6];
@@ -993,14 +1003,22 @@ static void store_last_used(struct a2dp_channel *chan, uint8_t lseid,
btd_adapter_get_storage_dir(device_get_adapter(chan->device)),
dst_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);
+ }

sprintf(value, "%02hhx:%02hhx", lseid, rseid);

g_key_file_set_string(key_file, "Endpoints", "LastUsed", value);

data = g_key_file_to_data(key_file, &len, NULL);
- g_file_set_contents(filename, data, len, NULL);
+ if (!g_file_set_contents(filename, data, len, &gerr)) {
+ error("Unable set contents for %s: (%s)", filename,
+ gerr->message);
+ g_error_free(gerr);
+ }

g_free(data);
g_key_file_free(key_file);
@@ -2218,6 +2236,7 @@ static void load_remote_seps(struct a2dp_channel *chan)
char dst_addr[18];
char **keys;
GKeyFile *key_file;
+ GError *gerr = NULL;

ba2str(device_get_address(device), dst_addr);

@@ -2225,7 +2244,11 @@ static void load_remote_seps(struct a2dp_channel *chan)
btd_adapter_get_storage_dir(device_get_adapter(device)),
dst_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);
+ }
keys = g_key_file_get_keys(key_file, "Endpoints", NULL, NULL);

load_remote_sep(chan, key_file, keys);
--
2.25.1

2021-10-18 17:32:45

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 9/9] peripheral: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
peripheral/main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/peripheral/main.c b/peripheral/main.c
index 6ce729178..86b52236e 100644
--- a/peripheral/main.c
+++ b/peripheral/main.c
@@ -77,7 +77,8 @@ static void prepare_filesystem(void)

if (lstat(mount_table[i].target, &st) < 0) {
printf("Creating %s\n", mount_table[i].target);
- mkdir(mount_table[i].target, 0755);
+ if (mkdir(mount_table[i].target, 0755) < 0)
+ perror("Failed to create dir");
}

printf("Mounting %s to %s\n", mount_table[i].fstype,
--
2.25.1

2021-10-18 17:32:46

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 1/9] device: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

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

2021-10-18 17:32:47

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 7/9] mesh: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
mesh/keyring.c | 6 ++++--
mesh/mesh-io-unit.c | 6 ++++--
mesh/rpl.c | 22 ++++++++++++++--------
mesh/util.c | 11 ++++++++---
4 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/mesh/keyring.c b/mesh/keyring.c
index 4b901643c..51621777d 100644
--- a/mesh/keyring.c
+++ b/mesh/keyring.c
@@ -50,7 +50,8 @@ static int open_key_file(struct mesh_node *node, const char *key_dir,

if (flags & O_CREAT) {
snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
- mkdir(fname, 0755);
+ if (mkdir(fname, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, fname);
}

snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
@@ -206,7 +207,8 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,

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

- mkdir(key_file, 0755);
+ if (mkdir(key_file, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, key_file);

for (i = 0; i < count; i++) {
snprintf(key_file, PATH_MAX, "%s%s/%4.4x", node_path,
diff --git a/mesh/mesh-io-unit.c b/mesh/mesh-io-unit.c
index c5aae6741..f4b615ac8 100644
--- a/mesh/mesh-io-unit.c
+++ b/mesh/mesh-io-unit.c
@@ -133,7 +133,8 @@ static bool incoming(struct l_io *sio, void *user_data)

buf[0] = 0;
memcpy(buf + 1, pvt->unique_name, size + 1);
- send(pvt->fd, buf, size + 2, MSG_DONTWAIT);
+ if (send(pvt->fd, buf, size + 2, MSG_DONTWAIT) < 0)
+ l_error("Failed to send(%d)", errno);
}

return true;
@@ -304,7 +305,8 @@ static bool simple_match(const void *a, const void *b)
static void send_pkt(struct mesh_io_private *pvt, struct tx_pkt *tx,
uint16_t interval)
{
- send(pvt->fd, tx->pkt, tx->len, MSG_DONTWAIT);
+ if (send(pvt->fd, tx->pkt, tx->len, MSG_DONTWAIT) < 0)
+ l_error("Failed to send(%d)", errno);

if (tx->delete) {
l_queue_remove_if(pvt->tx_pkts, simple_match, tx);
diff --git a/mesh/rpl.c b/mesh/rpl.c
index c53c6fbfd..9a99afe7b 100644
--- a/mesh/rpl.c
+++ b/mesh/rpl.c
@@ -18,6 +18,7 @@
#include <stdio.h>
#include <unistd.h>
#include <dirent.h>
+#include <errno.h>

#include <sys/stat.h>

@@ -54,9 +55,10 @@ bool rpl_put_entry(struct mesh_node *node, uint16_t src, uint32_t iv_index,
iv_index);
dir = opendir(src_file);

- if (!dir)
- mkdir(src_file, 0755);
- else
+ if (!dir) {
+ if (mkdir(src_file, 0755) != 0)
+ l_error("Failed to create dir: %s", src_file);
+ } else
closedir(dir);

snprintf(src_file, PATH_MAX, "%s%s/%8.8x/%4.4x", node_path, rpl_dir,
@@ -78,8 +80,8 @@ bool rpl_put_entry(struct mesh_node *node, uint16_t src, uint32_t iv_index,
iv_index--;
snprintf(src_file, PATH_MAX, "%s%s/%8.8x/%4.4x", node_path, rpl_dir,
iv_index, src);
- remove(src_file);
-
+ if (remove(src_file) < 0)
+ l_error("Failed to remove(%d): %s", errno, src_file);

return result;
}
@@ -110,7 +112,9 @@ void rpl_del_entry(struct mesh_node *node, uint16_t src)
if (entry->d_type == DT_DIR && entry->d_name[0] != '.') {
snprintf(rpl_path, PATH_MAX, "%s%s/%s/%4.4x",
node_path, rpl_dir, entry->d_name, src);
- remove(rpl_path);
+ if (remove(rpl_path) < 0)
+ l_error("Failed to remove(%d): %s", errno,
+ rpl_path);
}
}

@@ -251,7 +255,8 @@ void rpl_update(struct mesh_node *node, uint32_t cur)

/* Make sure path exists */
snprintf(path, PATH_MAX, "%s%s", node_path, rpl_dir);
- mkdir(path, 0755);
+ if (mkdir(path, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, path);

dir = opendir(path);
if (!dir)
@@ -288,6 +293,7 @@ bool rpl_init(const char *node_path)
return false;

snprintf(path, PATH_MAX, "%s%s", node_path, rpl_dir);
- mkdir(path, 0755);
+ if (mkdir(path, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, path);
return true;
}
diff --git a/mesh/util.c b/mesh/util.c
index 308e7d998..d505e7a0c 100644
--- a/mesh/util.c
+++ b/mesh/util.c
@@ -14,6 +14,7 @@

#define _GNU_SOURCE
#include <dirent.h>
+#include <errno.h>
#include <ftw.h>
#include <unistd.h>
#include <stdio.h>
@@ -117,12 +118,14 @@ int create_dir(const char *dir_name)
}

strncat(dir, prev + 1, next - prev);
- mkdir(dir, 0755);
+ if (mkdir(dir, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, dir);

prev = next;
}

- mkdir(dir_name, 0755);
+ if (mkdir(dir_name, 0755) != 0)
+ l_error("Failed to create dir(%d): %s", errno, dir_name);

return 0;
}
@@ -138,7 +141,9 @@ static int del_fobject(const char *fpath, const struct stat *sb, int typeflag,

case FTW_SL:
default:
- remove(fpath);
+ if (remove(fpath) < 0)
+ l_error("Failed to remove(%d): %s", errno, fpath);
+
l_debug("RM %s", fpath);
break;
}
--
2.25.1

2021-10-18 18:03:01

by bluez.test.bot

[permalink] [raw]
Subject: RE: Fix unchecked return value

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=565705

---Test result---

Test Summary:
CheckPatch PASS 14.20 seconds
GitLint PASS 8.79 seconds
Prep - Setup ELL PASS 50.33 seconds
Build - Prep PASS 0.53 seconds
Build - Configure PASS 9.44 seconds
Build - Make PASS 219.25 seconds
Make Check PASS 9.97 seconds
Make Distcheck PASS 282.41 seconds
Build w/ext ELL - Configure PASS 10.45 seconds
Build w/ext ELL - Make PASS 223.90 seconds



---
Regards,
Linux Bluetooth

2021-10-18 18:30:36

by Tedd Ho-Jeong An

[permalink] [raw]
Subject: [BlueZ PATCH 4/9] plugins/admin: Fix unchecked return value

From: Tedd Ho-Jeong An <[email protected]>

This patch fixes the unchecked return value(CWE-252) issues reported by
the Coverity.
---
plugins/admin.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/plugins/admin.c b/plugins/admin.c
index 7b7190a06..a8e7d2cd7 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -247,6 +247,7 @@ failed:
static void store_policy_settings(struct btd_admin_policy *admin_policy)
{
GKeyFile *key_file = NULL;
+ GError *gerr = NULL;
char *filename = ADMIN_POLICY_STORAGE;
char *key_file_data = NULL;
char **uuid_strs = NULL;
@@ -274,7 +275,12 @@ static void store_policy_settings(struct btd_admin_policy *admin_policy)
}

key_file_data = g_key_file_to_data(key_file, &length, NULL);
- g_file_set_contents(ADMIN_POLICY_STORAGE, key_file_data, length, NULL);
+ if (!g_file_set_contents(ADMIN_POLICY_STORAGE, key_file_data, length,
+ &gerr)) {
+ error("Unable set contents for %s: (%s)", ADMIN_POLICY_STORAGE,
+ gerr->message);
+ g_error_free(gerr);
+ }

g_free(key_file_data);
free_uuid_strings(uuid_strs, num_uuids);
@@ -335,6 +341,7 @@ failed:
static void load_policy_settings(struct btd_admin_policy *admin_policy)
{
GKeyFile *key_file;
+ GError *gerr = NULL;
char *filename = ADMIN_POLICY_STORAGE;
struct stat st;

@@ -343,7 +350,11 @@ static void load_policy_settings(struct btd_admin_policy *admin_policy)

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);
+ }

key_file_load_service_allowlist(key_file, admin_policy);

--
2.25.1