RE: Fix bugs found by static analysis

RE: Fix bugs found by static analysis

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 7677 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.21 seconds
GitLint                       PENDING   0.28 seconds
BuildEll                      PASS      20.46 seconds
BluezMake                     FAIL      19.04 seconds
MakeCheck                     FAIL      33.43 seconds
MakeDistcheck                 PASS      182.31 seconds
CheckValgrind                 FAIL      15.70 seconds
CheckSmatch                   FAIL      22.20 seconds
bluezmakeextell               FAIL      13.10 seconds
IncrementalBuild              PENDING   0.24 seconds
ScanBuild                     FAIL      22.64 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:

src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7469: src/shared/libshared_mainloop_la-bap.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4030: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7224: src/shared/libshared_glib_la-bap.lo] Error 1
make: *** [Makefile:10435: check] Error 2
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7469: src/shared/libshared_mainloop_la-bap.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:10435: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:2612:9: warning: mixing declarations and code
src/shared/bap.c:317:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/shell.c: note: in included file (through /usr/include/readline/readline.h):
/usr/include/readline/rltypedefs.h:35:23: warning: non-ANSI function declaration of function 'Function'
/usr/include/readline/rltypedefs.h:36:25: warning: non-ANSI function declaration of function 'VFunction'
/usr/include/readline/rltypedefs.h:37:27: warning: non-ANSI function declaration of function 'CPFunction'
/usr/include/readline/rltypedefs.h:38:29: warning: non-ANSI function declaration of function 'CPPFunction'
src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7469: src/shared/libshared_mainloop_la-bap.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4030: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:

src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7469: src/shared/libshared_mainloop_la-bap.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4030: all] Error 2
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:

##############################
Test: ScanBuild - FAIL
Desc: Run Scan Build
Output:

src/shared/bap.c: In function ‘bap_bcast_io_dir’:
src/shared/bap.c:2612:2: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
 2612 |  uint8_t dir;
      |  ^~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:7469: src/shared/libshared_mainloop_la-bap.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
src/shared/gatt-client.c:451:21: warning: Use of memory after it is freed
        gatt_db_unregister(op->client->db, op->db_id);
                           ^~~~~~~~~~
src/shared/gatt-client.c:696:2: warning: Use of memory after it is freed
        discovery_op_complete(op, false, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:996:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1102:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1296:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1361:2: warning: Use of memory after it is freed
        discovery_op_complete(op, success, att_ecode);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1636:6: warning: Use of memory after it is freed
        if (read_db_hash(op)) {
            ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:1641:2: warning: Use of memory after it is freed
        discover_all(op);
        ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2147:6: warning: Use of memory after it is freed
        if (read_db_hash(op)) {
            ^~~~~~~~~~~~~~~~
src/shared/gatt-client.c:2155:8: warning: Use of memory after it is freed
                                                        discovery_op_ref(op),
                                                        ^~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3180:2: warning: Use of memory after it is freed
        complete_write_long_op(req, success, 0, false);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/shared/gatt-client.c:3202:2: warning: Use of memory after it is freed
        request_unref(req);
        ^~~~~~~~~~~~~~~~~~
12 warnings generated.
make: *** [Makefile:4030: all] Error 2


---
Regards,
Linux Bluetooth

[PATCH BlueZ v2 00/11] Fix bugs found by static analysis

[Ismagil Iskakov]

This patch corrects some of the bugs not connected
to the functionality but to memory management etc.
Two exceptions being about fixing the arguments
order, which also could've caused some trouble.

Ismagil Iskakov (11):
  btio: fix range validation of security level
  profiles/audio: add nullity checks
  src/shared: add nullity checks
  isotest: close fd after sending when nconn=1
  obexd/client: fix err condition causing memleak
  profiles/audio: fix memleak of bt_bap
  src/shared: fix memleak
  src/shared: move null checks before dereferencing
  isotest: remove repeating conditions
  profiles/audio: fix io_unlink args order
  src/plugin: fix args order

 btio/btio.c                |  2 +-
 obexd/client/transfer.c    |  2 +-
 profiles/audio/a2dp.c      | 45 +++++++++++++++++++++++++++++---------
 profiles/audio/avrcp.c     | 24 +++++++++++++++++---
 profiles/audio/bap.c       | 24 +++++++++++---------
 profiles/audio/bass.c      |  7 +++++-
 profiles/audio/transport.c |  2 +-
 src/plugin.c               |  2 +-
 src/shared/bap.c           | 40 ++++++++++++++++++++++++++++-----
 src/shared/gatt-db.c       |  5 ++++-
 src/shared/vcp.c           |  3 +++
 tools/isotest.c            |  8 ++++---
 12 files changed, 126 insertions(+), 38 deletions(-)

-- 
2.34.1

[PATCH BlueZ v2 01/11] btio: fix range validation of security level

[Ismagil Iskakov]

Arrays inside l2cap_set_lm/rfcomm_set_lm functions are of size 4,
but the bounds check allows the value 4 for 'level'.
---
 btio/btio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/btio/btio.c b/btio/btio.c
index b8afe0580..14f2b700e 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -455,7 +455,7 @@ static gboolean set_sec_level(int sock, BtIOType type, int level, GError **err)
 	struct bt_security sec;
 	int ret;
 
-	if (level < BT_SECURITY_LOW || level > BT_SECURITY_FIPS) {
+	if (level < BT_SECURITY_LOW || level > BT_SECURITY_HIGH) {
 		g_set_error(err, BT_IO_ERROR, EINVAL,
 				"Valid security level range is %d-%d",
 				BT_SECURITY_LOW, BT_SECURITY_HIGH);
-- 
2.34.1

[PATCH BlueZ v2 02/11] profiles/audio: add nullity checks

[Ismagil Iskakov]

Cover bass_setup unsuccessful search and btd_device_get_service.
This change is motivated by the other usages where checks for
NULL exist.
---
 profiles/audio/a2dp.c  | 45 ++++++++++++++++++++++++++++++++----------
 profiles/audio/avrcp.c | 24 +++++++++++++++++++---
 profiles/audio/bass.c  |  5 +++++
 3 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index 6204006d6..56a035c7e 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -646,6 +646,24 @@ static gboolean auto_config(gpointer data)
 	struct btd_service *service;
 	struct a2dp_stream *stream;
 
+	dev = avdtp_get_device(setup->session);
+
+	if (setup->sep->type == AVDTP_SEP_TYPE_SOURCE) {
+		service = btd_device_get_service(dev, A2DP_SINK_UUID);
+
+		if (service == NULL) {
+			error("a2dp sink service not found");
+			return FALSE;
+		}
+	} else {
+		service = btd_device_get_service(dev, A2DP_SOURCE_UUID);
+
+		if (service == NULL) {
+			error("a2dp source service not found");
+			return FALSE;
+		}
+	}
+
 	/* Check if configuration was aborted */
 	stream = queue_find(setup->sep->streams, match_stream, setup->stream);
 	if (!stream)
@@ -654,16 +672,12 @@ static gboolean auto_config(gpointer data)
 	if (setup->err != NULL)
 		goto done;
 
-	dev = avdtp_get_device(setup->session);
-
 	avdtp_stream_add_cb(setup->session, setup->stream,
 				stream_state_changed, setup->sep);
 
 	if (setup->sep->type == AVDTP_SEP_TYPE_SOURCE) {
-		service = btd_device_get_service(dev, A2DP_SINK_UUID);
 		sink_new_stream(service, setup->session, setup->stream);
 	} else {
-		service = btd_device_get_service(dev, A2DP_SOURCE_UUID);
 		source_new_stream(service, setup->session, setup->stream);
 	}
 
@@ -995,10 +1009,25 @@ static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	struct btd_service *service;
 	int ret;
 
-	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK)
+	dev = avdtp_get_device(session);
+
+	if (a2dp_sep->type == AVDTP_SEP_TYPE_SINK) {
 		DBG("Sink %p: Set_Configuration_Cfm", sep);
-	else
+		service = btd_device_get_service(dev, A2DP_SOURCE_UUID);
+
+		if (service == NULL) {
+			error("a2dp source service not found");
+			return;
+		}
+	} else {
 		DBG("Source %p: Set_Configuration_Cfm", sep);
+		service = btd_device_get_service(dev, A2DP_SINK_UUID);
+
+		if (service == NULL) {
+			error("a2dp sink service not found");
+			return;
+		}
+	}
 
 	setup = find_setup_by_session(session);
 
@@ -1024,14 +1053,10 @@ static void setconf_cfm(struct avdtp *session, struct avdtp_local_sep *sep,
 	if (!setup)
 		return;
 
-	dev = avdtp_get_device(session);
-
 	/* Notify D-Bus interface of the new stream */
 	if (a2dp_sep->type == AVDTP_SEP_TYPE_SOURCE) {
-		service = btd_device_get_service(dev, A2DP_SINK_UUID);
 		sink_new_stream(service, session, setup->stream);
 	} else {
-		service = btd_device_get_service(dev, A2DP_SOURCE_UUID);
 		source_new_stream(service, session, setup->stream);
 	}
 
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index ba191e441..08edeac40 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -3082,8 +3082,14 @@ static void set_ct_player(struct avrcp *session, struct avrcp_player *player)
 	if (session->controller->player == player)
 		goto done;
 
-	session->controller->player = player;
 	service = btd_device_get_service(session->dev, AVRCP_TARGET_UUID);
+
+	if (service == NULL) {
+		error("avrcp target service not found");
+		return;
+	}
+
+	session->controller->player = player;
 	control_set_player(service, player ?
 			media_player_get_path(player->user_data) : NULL);
 
@@ -4278,12 +4284,18 @@ static void target_init(struct avrcp *session)
 	if (session->target != NULL)
 		return;
 
+	service = btd_device_get_service(session->dev, AVRCP_REMOTE_UUID);
+
+	if (service == NULL) {
+		error("avrcp remote service not found");
+		return;
+	}
+
 	target = data_init(session, AVRCP_REMOTE_UUID);
 	session->target = target;
 
 	DBG("%p version 0x%04x", target, target->version);
 
-	service = btd_device_get_service(session->dev, AVRCP_REMOTE_UUID);
 	btd_service_connecting_complete(service, 0);
 
 	player = g_slist_nth_data(server->players, 0);
@@ -4332,6 +4344,13 @@ static void controller_init(struct avrcp *session)
 	if (session->controller != NULL)
 		return;
 
+	service = btd_device_get_service(session->dev, AVRCP_TARGET_UUID);
+
+	if (service == NULL) {
+		error("avrcp target service not found");
+		return;
+	}
+	
 	controller = data_init(session, AVRCP_TARGET_UUID);
 	session->controller = controller;
 
@@ -4339,7 +4358,6 @@ static void controller_init(struct avrcp *session)
 	if (controller->obex_port)
 		DBG("%p OBEX PSM 0x%04x", controller, controller->obex_port);
 
-	service = btd_device_get_service(session->dev, AVRCP_TARGET_UUID);
 	btd_service_connecting_complete(service, 0);
 
 	/* Only create player if category 1 is supported */
diff --git a/profiles/audio/bass.c b/profiles/audio/bass.c
index b27a3fc12..f617efa2c 100644
--- a/profiles/audio/bass.c
+++ b/profiles/audio/bass.c
@@ -349,6 +349,11 @@ static void bap_state_changed(struct bt_bap_stream *stream, uint8_t old_state,
 	struct bass_setup *setup = queue_find(dg->setups,
 				match_setup_stream, stream);
 
+	if (setup == NULL) {
+		error("unable to find setup in delegator");
+		return;
+	}
+
 	if (dg->bap != bap)
 		return;
 
-- 
2.34.1

[PATCH BlueZ v2 03/11] src/shared: add nullity checks

[Ismagil Iskakov]

Check util_iov_pull_mem where iov len is not verified
beforehand. Check vcp_get_vcs for NULL.
These changes are based on other usages where those
checks exist.
---
 src/shared/bap.c | 23 +++++++++++++++++++++++
 src/shared/vcp.c |  3 +++
 2 files changed, 26 insertions(+)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 76340d565..a866f4cdc 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -7457,6 +7457,11 @@ bool bt_bap_parse_base(uint8_t sid, struct iovec *iov,
 
 		codec = util_iov_pull_mem(iov, sizeof(*codec));
 
+		if (!codec) {
+			ret = false;
+			goto done;
+		}
+
 		util_debug(func, NULL, "Codec: ID %d CID 0x%2.2x VID 0x%2.2x",
 				codec->id, codec->cid, codec->vid);
 
@@ -7468,6 +7473,12 @@ bool bt_bap_parse_base(uint8_t sid, struct iovec *iov,
 		}
 
 		l2_cc.iov_base = util_iov_pull_mem(iov, l2_cc_len);
+
+		if (!l2_cc.iov_base) {
+			ret = false;
+			goto done;
+		}
+
 		l2_cc.iov_len = l2_cc_len;
 
 		/* Print Codec Specific Configuration */
@@ -7482,6 +7493,12 @@ bool bt_bap_parse_base(uint8_t sid, struct iovec *iov,
 		}
 
 		meta.iov_base = util_iov_pull_mem(iov, meta_len);
+
+		if (!meta.iov_base) {
+			ret = false;
+			goto done;
+		}
+
 		meta.iov_len = meta_len;
 
 		/* Print Metadata */
@@ -7512,6 +7529,12 @@ bool bt_bap_parse_base(uint8_t sid, struct iovec *iov,
 
 			l3_cc.iov_base = util_iov_pull_mem(iov,
 							l3_cc_len);
+
+			if (!l3_cc.iov_base) {
+				ret = false;
+				goto done;
+			}
+			
 			l3_cc.iov_len = l3_cc_len;
 
 			/* Print Codec Specific Configuration */
diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index c96ad4376..63acaefb4 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -2934,6 +2934,9 @@ static void foreach_vcs_service(struct gatt_db_attribute *attr,
 	struct bt_vcp *vcp = user_data;
 	struct bt_vcs *vcs = vcp_get_vcs(vcp);
 
+	if (!vcs)
+		return;
+
 	vcs->service = attr;
 
 	gatt_db_service_set_claimed(attr, true);
-- 
2.34.1

[PATCH BlueZ v2 04/11] isotest: close fd after sending when nconn=1

[Ismagil Iskakov]

---
 tools/isotest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/isotest.c b/tools/isotest.c
index 2cac0e49c..0ced19a9e 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -973,6 +973,8 @@ static void send_mode(char *filename, char *peer, int i, bool repeat)
 	}
 
 	do_send(sk, fd, peer, repeat);
+	if (fd >= 0)
+		close(fd);
 }
 
 static void reconnect_mode(char *peer)
-- 
2.34.1

[PATCH BlueZ v2 05/11] obexd/client: fix err condition causing memleak

[Ismagil Iskakov]

transfer_open returns 0 if an error occurs, condition corrected.
---
 obexd/client/transfer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
index a7d00896f..b078c1f6c 100644
--- a/obexd/client/transfer.c
+++ b/obexd/client/transfer.c
@@ -556,7 +556,7 @@ struct obc_transfer *obc_transfer_get(const char *type, const char *name,
 	transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type);
 
 	perr = transfer_open(transfer, O_WRONLY | O_CREAT | O_TRUNC, 0600, err);
-	if (perr < 0) {
+	if (perr == FALSE) {
 		obc_transfer_free(transfer);
 		return NULL;
 	}
-- 
2.34.1

[PATCH BlueZ v2 06/11] profiles/audio: fix memleak of bt_bap

[Ismagil Iskakov]

Make failure branches deallocate memory before leaving.
---
 profiles/audio/bap.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index ee7c8bc49..ba9000183 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -2954,32 +2954,36 @@ static int bap_bcast_probe(struct btd_service *service)
 	struct btd_adapter *adapter = device_get_adapter(device);
 	struct btd_gatt_database *database = btd_adapter_get_database(adapter);
 	struct bap_data *data;
+	struct bt_bap *bap;
 
 	if (!btd_adapter_has_exp_feature(adapter, EXP_FEAT_ISO_SOCKET)) {
 		error("BAP requires ISO Socket which is not enabled");
 		return -ENOTSUP;
 	}
 
-	data = bap_data_new(device);
-	data->service = service;
-	data->adapter = adapter;
-	data->device = device;
-	data->bap = bt_bap_new(btd_gatt_database_get_db(database),
+	bap = bt_bap_new(btd_gatt_database_get_db(database),
 			btd_gatt_database_get_db(database));
-	if (!data->bap) {
+
+	if (!bap) {
 		error("Unable to create BAP instance");
-		free(data);
 		return -EINVAL;
 	}
-	data->bcast_snks = queue_new();
 
-	bt_bap_set_user_data(data->bap, service);
+	bt_bap_set_user_data(bap, service);
 
-	if (!bt_bap_attach(data->bap, NULL)) {
+	if (!bt_bap_attach(bap, NULL)) {
 		error("BAP unable to attach");
+		bt_bap_unref(bap);
 		return -EINVAL;
 	}
 
+	data = bap_data_new(device);
+	data->service = service;
+	data->adapter = adapter;
+	data->device = device;
+	data->bap = bap;
+	data->bcast_snks = queue_new();
+
 	bap_data_add(data);
 
 	data->ready_id = bt_bap_ready_register(data->bap, bap_ready, service,
-- 
2.34.1

[PATCH BlueZ v2 07/11] src/shared: fix memleak

[Ismagil Iskakov]

---
 src/shared/bap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index a866f4cdc..96fca595b 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -6910,14 +6910,15 @@ static void add_new_subgroup(struct bt_base *base,
 			struct bt_bap_stream *stream)
 {
 	struct bt_bap_pac *lpac = stream->lpac;
-	struct bt_subgroup *sgrp = new0(
-				struct bt_subgroup, 1);
+	struct bt_subgroup *sgrp;
 	uint16_t cid = 0;
 	uint16_t vid = 0;
 
 	if (!lpac)
 		return;
 
+	sgrp = new0(struct bt_subgroup, 1);
+
 	bt_bap_pac_get_vendor_codec(lpac, &sgrp->codec.id, &cid,
 			&vid, NULL, NULL);
 	sgrp->codec.cid = cid;
-- 
2.34.1

[PATCH BlueZ v2 08/11] src/shared: move null checks before dereferencing

[Ismagil Iskakov]

---
 src/shared/bap.c     | 12 ++++++++----
 src/shared/gatt-db.c |  5 ++++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/src/shared/bap.c b/src/shared/bap.c
index 96fca595b..2ddd6e1d1 100644
--- a/src/shared/bap.c
+++ b/src/shared/bap.c
@@ -2571,11 +2571,13 @@ static uint8_t bap_ucast_io_dir(struct bt_bap_stream *stream)
 static uint8_t bap_bcast_io_dir(struct bt_bap_stream *stream)
 {
 	uint8_t dir;
-	uint8_t pac_type = bt_bap_pac_get_type(stream->lpac);
+	uint8_t pac_type;
 
 	if (!stream)
 		return 0x00;
 
+	pac_type = bt_bap_pac_get_type(stream->lpac);
+
 	if (pac_type == BT_BAP_BCAST_SINK)
 		dir = BT_BAP_BCAST_SOURCE;
 	else
@@ -6144,7 +6146,7 @@ static struct bt_bap_stream *bap_bcast_stream_new(struct bt_bap *bap,
 	struct bt_bap_endpoint *ep = NULL;
 	struct match_pac match;
 
-	if (!bap)
+	if (!bap || !lpac)
 		return NULL;
 
 	if (lpac->type == BT_BAP_BCAST_SOURCE) {
@@ -6153,7 +6155,7 @@ static struct bt_bap_stream *bap_bcast_stream_new(struct bt_bap *bap,
 		memset(&match.codec, 0, sizeof(match.codec));
 
 		bt_bap_foreach_pac(bap, BT_BAP_BCAST_SINK, match_pac, &match);
-		if ((!match.lpac) || (!lpac))
+		if (!match.lpac)
 			return NULL;
 
 		lpac = match.lpac;
@@ -6406,11 +6408,13 @@ unsigned int bt_bap_stream_release(struct bt_bap_stream *stream,
 					void *user_data)
 {
 	unsigned int id;
-	struct bt_bap *bap = stream->bap;
+	struct bt_bap *bap;
 
 	if (!stream || !stream->ops || !stream->ops->release)
 		return 0;
 
+	bap = stream->bap;
+
 	if (!bt_bap_ref_safe(bap))
 		return 0;
 
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index 8951079be..a4fa8aed9 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -1391,12 +1391,15 @@ static void find_by_type(struct gatt_db_attribute *attribute, void *user_data)
 {
 	struct find_by_type_value_data *search_data = user_data;
 
+	if (!attribute)
+		return;
+
 	/* TODO: fix for read-callback based attributes */
 	if (search_data->value) {
 		if (search_data->value_len != attribute->value_len)
 			return;
 
-		if (!attribute || !attribute->value)
+		if (!attribute->value)
 			return;
 
 		if (memcmp(attribute->value, search_data->value,
-- 
2.34.1

[PATCH BlueZ v2 09/11] isotest: remove repeating conditions

[Ismagil Iskakov]

---
 tools/isotest.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/isotest.c b/tools/isotest.c
index 0ced19a9e..924876d35 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -684,9 +684,9 @@ static void recv_mode(int fd, int sk, char *peer)
 
 			r = recv(sk, buf, data_size, 0);
 			if (r < 0) {
-				if (r < 0)
-					syslog(LOG_ERR, "Read failed: %s (%d)",
-							strerror(errno), errno);
+				syslog(LOG_ERR, "Read failed: %s (%d)",
+						strerror(errno), errno);
+
 				if (errno != ENOTCONN)
 					return;
 
-- 
2.34.1

[PATCH BlueZ v2 10/11] profiles/audio: fix io_unlink args order

[Ismagil Iskakov]

Seems like a breaking bug. Analogous to bt_bap_stream_io_link,
but there the order is correct.
---
 profiles/audio/bass.c      | 2 +-
 profiles/audio/transport.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/bass.c b/profiles/audio/bass.c
index f617efa2c..e1c05e05a 100644
--- a/profiles/audio/bass.c
+++ b/profiles/audio/bass.c
@@ -457,7 +457,7 @@ static void stream_unlink(void *data, void *user_data)
 	struct bt_bap_stream *link = data;
 	struct bt_bap_stream *stream = user_data;
 
-	bt_bap_stream_io_unlink(link, stream);
+	bt_bap_stream_io_unlink(stream, link);
 }
 
 static void bass_remove_bis(struct bass_setup *setup)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index a1fdf948b..a355bde24 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -2015,7 +2015,7 @@ static void transport_unlink(void *data, void *user_data)
 		return;
 	}
 
-	bt_bap_stream_io_unlink(link, stream);
+	bt_bap_stream_io_unlink(stream, link);
 
 	bap_update_links(transport);
 
-- 
2.34.1

[PATCH BlueZ v2 11/11] src/plugin: fix args order

[Ismagil Iskakov]

According to other functions and arguments usage, the
case with external_plugin_init seems to be erroneous.
---
 src/plugin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/plugin.c b/src/plugin.c
index a566bd2f4..dcdb1b2db 100644
--- a/src/plugin.c
+++ b/src/plugin.c
@@ -141,7 +141,7 @@ static gboolean enable_plugin(const char *name, char **cli_enable,
 }
 
 
-static void external_plugin_init(char **cli_disabled, char **cli_enabled)
+static void external_plugin_init(char **cli_enabled, char **cli_disabled)
 {
 	GDir *dir;
 	const char *file;
-- 
2.34.1

RE: Fix bugs found by static analysis

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.23 seconds
BuildEll                      PASS      20.07 seconds
BluezMake                     PASS      2677.72 seconds
MakeCheck                     PASS      20.01 seconds
MakeDistcheck                 PASS      184.57 seconds
CheckValgrind                 PASS      233.46 seconds
CheckSmatch                   WARNING   303.91 seconds
bluezmakeextell               PASS      128.51 seconds
IncrementalBuild              PENDING   0.25 seconds
ScanBuild                     PASS      935.76 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ v2 01/11] btio: fix range validation of security level

[Luiz Augusto von Dentz]

Hi Ismagil,

On Tue, Jul 8, 2025 at 7:13 AM Ismagil Iskakov <i.iskakov@omp.ru> wrote:
>
> Arrays inside l2cap_set_lm/rfcomm_set_lm functions are of size 4,
> but the bounds check allows the value 4 for 'level'.
> ---
>  btio/btio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index b8afe0580..14f2b700e 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -455,7 +455,7 @@ static gboolean set_sec_level(int sock, BtIOType type, int level, GError **err)
>         struct bt_security sec;
>         int ret;
>
> -       if (level < BT_SECURITY_LOW || level > BT_SECURITY_FIPS) {
> +       if (level < BT_SECURITY_LOW || level > BT_SECURITY_HIGH) {
>                 g_set_error(err, BT_IO_ERROR, EINVAL,
>                                 "Valid security level range is %d-%d",
>                                 BT_SECURITY_LOW, BT_SECURITY_HIGH);
> --
> 2.34.1

Yeah, while this handles the likes of l2cap_set_lm/rfcomm_set_lm this
also would disallow FIPS (level 4) security, the right fix here is to
probably bail out with an error after attempting BT_SECURITY if
BT_SECURITY_FIPS has been given.


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v2 05/11] obexd/client: fix err condition causing memleak

[Luiz Augusto von Dentz]

Hi Ismagil,

On Tue, Jul 8, 2025 at 7:11 AM Ismagil Iskakov <i.iskakov@omp.ru> wrote:
>
> transfer_open returns 0 if an error occurs, condition corrected.

It actually returns a boolean.

> ---
>  obexd/client/transfer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/obexd/client/transfer.c b/obexd/client/transfer.c
> index a7d00896f..b078c1f6c 100644
> --- a/obexd/client/transfer.c
> +++ b/obexd/client/transfer.c
> @@ -556,7 +556,7 @@ struct obc_transfer *obc_transfer_get(const char *type, const char *name,
>         transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type);
>
>         perr = transfer_open(transfer, O_WRONLY | O_CREAT | O_TRUNC, 0600, err);
> -       if (perr < 0) {
> +       if (perr == FALSE) {

if (!perr)

>                 obc_transfer_free(transfer);
>                 return NULL;
>         }
> --
> 2.34.1
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v2 10/11] profiles/audio: fix io_unlink args order

[Luiz Augusto von Dentz]

Hi Ismagil,

On Tue, Jul 8, 2025 at 7:12 AM Ismagil Iskakov <i.iskakov@omp.ru> wrote:
>
> Seems like a breaking bug. Analogous to bt_bap_stream_io_link,
> but there the order is correct.

The order doesn't change anything, the exact same operations are
performed on both parameters, we could just name it stream and stream1
instead of link if that confuses the static analyzer, that said I
don't think that should use the argument names are indication of the
semantics of the function, specially since it can see the underline
code.

> ---
>  profiles/audio/bass.c      | 2 +-
>  profiles/audio/transport.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/profiles/audio/bass.c b/profiles/audio/bass.c
> index f617efa2c..e1c05e05a 100644
> --- a/profiles/audio/bass.c
> +++ b/profiles/audio/bass.c
> @@ -457,7 +457,7 @@ static void stream_unlink(void *data, void *user_data)
>         struct bt_bap_stream *link = data;
>         struct bt_bap_stream *stream = user_data;
>
> -       bt_bap_stream_io_unlink(link, stream);
> +       bt_bap_stream_io_unlink(stream, link);
>  }
>
>  static void bass_remove_bis(struct bass_setup *setup)
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index a1fdf948b..a355bde24 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -2015,7 +2015,7 @@ static void transport_unlink(void *data, void *user_data)
>                 return;
>         }
>
> -       bt_bap_stream_io_unlink(link, stream);
> +       bt_bap_stream_io_unlink(stream, link);
>
>         bap_update_links(transport);
>
> --
> 2.34.1
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ v2 00/11] Fix bugs found by static analysis

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Tue, 8 Jul 2025 14:08:58 +0300 you wrote:
> This patch corrects some of the bugs not connected
> to the functionality but to memory management etc.
> Two exceptions being about fixing the arguments
> order, which also could've caused some trouble.
> 
> Ismagil Iskakov (11):
>   btio: fix range validation of security level
>   profiles/audio: add nullity checks
>   src/shared: add nullity checks
>   isotest: close fd after sending when nconn=1
>   obexd/client: fix err condition causing memleak
>   profiles/audio: fix memleak of bt_bap
>   src/shared: fix memleak
>   src/shared: move null checks before dereferencing
>   isotest: remove repeating conditions
>   profiles/audio: fix io_unlink args order
>   src/plugin: fix args order
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,01/11] btio: fix range validation of security level
    (no matching commit)
  - [BlueZ,v2,02/11] profiles/audio: add nullity checks
    (no matching commit)
  - [BlueZ,v2,03/11] src/shared: add nullity checks
    (no matching commit)
  - [BlueZ,v2,04/11] isotest: close fd after sending when nconn=1
    (no matching commit)
  - [BlueZ,v2,05/11] obexd/client: fix err condition causing memleak
    (no matching commit)
  - [BlueZ,v2,06/11] profiles/audio: fix memleak of bt_bap
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c1d4c478c40d
  - [BlueZ,v2,07/11] src/shared: fix memleak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f6dcd1d2bd71
  - [BlueZ,v2,08/11] src/shared: move null checks before dereferencing
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=47ffe6086f37
  - [BlueZ,v2,09/11] isotest: remove repeating conditions
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a2d7be18ace6
  - [BlueZ,v2,10/11] profiles/audio: fix io_unlink args order
    (no matching commit)
  - [BlueZ,v2,11/11] src/plugin: fix args order
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=a7e0747e21de

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH BlueZ v2 00/11] Fix bugs found by static analysis

[Luiz Augusto von Dentz]

Hi Ismagil,

On Tue, Jul 8, 2025 at 7:11 AM Ismagil Iskakov <i.iskakov@omp.ru> wrote:
>
> This patch corrects some of the bugs not connected
> to the functionality but to memory management etc.
> Two exceptions being about fixing the arguments
> order, which also could've caused some trouble.
>
> Ismagil Iskakov (11):
>   btio: fix range validation of security level
>   profiles/audio: add nullity checks
>   src/shared: add nullity checks
>   isotest: close fd after sending when nconn=1
>   obexd/client: fix err condition causing memleak
>   profiles/audio: fix memleak of bt_bap
>   src/shared: fix memleak
>   src/shared: move null checks before dereferencing
>   isotest: remove repeating conditions
>   profiles/audio: fix io_unlink args order
>   src/plugin: fix args order
>
>  btio/btio.c                |  2 +-
>  obexd/client/transfer.c    |  2 +-
>  profiles/audio/a2dp.c      | 45 +++++++++++++++++++++++++++++---------
>  profiles/audio/avrcp.c     | 24 +++++++++++++++++---
>  profiles/audio/bap.c       | 24 +++++++++++---------
>  profiles/audio/bass.c      |  7 +++++-
>  profiles/audio/transport.c |  2 +-
>  src/plugin.c               |  2 +-
>  src/shared/bap.c           | 40 ++++++++++++++++++++++++++++-----
>  src/shared/gatt-db.c       |  5 ++++-
>  src/shared/vcp.c           |  3 +++
>  tools/isotest.c            |  8 ++++---
>  12 files changed, 126 insertions(+), 38 deletions(-)
>
> --
> 2.34.1
>

When sending this type of change please include the tool output in the
patch description, anyway I went ahead and applied the one that seems
clear enough.


-- 
Luiz Augusto von Dentz

RE: Fix bugs found by static analysis

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.27 seconds
BuildEll                      PASS      20.71 seconds
BluezMake                     PASS      2771.19 seconds
MakeCheck                     PASS      20.05 seconds
MakeDistcheck                 PASS      184.73 seconds
CheckValgrind                 PASS      235.29 seconds
CheckSmatch                   WARNING   307.39 seconds
bluezmakeextell               PASS      129.11 seconds
IncrementalBuild              PENDING   0.26 seconds
ScanBuild                     PASS      934.68 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

RE: Fix bugs found by static analysis

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1864 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.27 seconds
GitLint                       PENDING   0.29 seconds
BuildEll                      PASS      22.59 seconds
BluezMake                     PASS      2749.28 seconds
MakeCheck                     PASS      20.24 seconds
MakeDistcheck                 PASS      189.54 seconds
CheckValgrind                 PASS      241.91 seconds
CheckSmatch                   WARNING   315.40 seconds
bluezmakeextell               PASS      130.05 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      909.24 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structuressrc/shared/bap.c:317:25: warning: array of flexible structuressrc/shared/bap.c: note: in included file:./src/shared/ascs.h:88:25: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth