public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [BlueZ v2 00/20] Fix a number of static analysis issues
@ 2024-05-10 12:10 Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 01/20] adapter: Use false instead of 0 for bool Bastien Nocera
                   ` (20 more replies)
  0 siblings, 21 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Changes since v1:
- added 6 patches
- Fix syntax error in "client/gatt: Check write_value() retval"

Bastien Nocera (20):
  adapter: Use false instead of 0 for bool
  attrib/gatt: Guard against possible integer overflow
  client/gatt: Don't pass negative fd on error
  client/gatt: Check write_value() retval
  client/main: Fix array access
  client/main: Fix mismatched free
  monitor/att: Fix memory leak
  bap: Fix memory leaks
  media: Fix memory leak
  main: Fix memory leaks
  isotest: Consider "0" fd to be valid
  isotest: Fix error check after opening file
  client/player: Fix copy/paste error
  shared/vcp: Fix copy/paste error
  isotest: Fix fd leak
  iso-tester: Fix fd leak
  sdp: Fix use of uninitialised memory
  monitor: Work-around memory leak warning
  avrcp: Fix uninitialised memory usage
  main: Simplify variable assignment

 attrib/gatt.c          |  8 ++++---
 client/gatt.c          | 21 +++++++++++++++----
 client/main.c          |  7 ++++++-
 client/player.c        |  2 +-
 lib/sdp.c              |  2 +-
 monitor/att.c          | 19 +++++++++++++++++
 monitor/jlink.c        |  3 ++-
 profiles/audio/avrcp.c | 10 ++++-----
 profiles/audio/bap.c   | 47 +++++++++++++++++++++++++++++-------------
 profiles/audio/media.c |  1 +
 src/adapter.c          |  2 +-
 src/main.c             | 16 +++++++-------
 src/shared/vcp.c       |  2 +-
 tools/iso-tester.c     |  1 +
 tools/isotest.c        |  6 ++++--
 15 files changed, 104 insertions(+), 43 deletions(-)

-- 
2.44.0


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [BlueZ v2 01/20] adapter: Use false instead of 0 for bool
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 02/20] attrib/gatt: Guard against possible integer overflow Bastien Nocera
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

---
 src/adapter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5505edbb29c1..8b478e213cb5 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2413,7 +2413,7 @@ static int update_discovery_filter(struct btd_adapter *adapter)
 	 * starting discovery.
 	 */
 	if (filters_equal(adapter->current_discovery_filter, sd_cp) &&
-	    adapter->discovering != 0) {
+	    adapter->discovering != false) {
 		DBG("filters were equal, deciding to not restart the scan.");
 		g_free(sd_cp);
 		return 0;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 02/20] attrib/gatt: Guard against possible integer overflow
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 01/20] adapter: Use false instead of 0 for bool Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 03/20] client/gatt: Don't pass negative fd on error Bastien Nocera
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: INTEGER_OVERFLOW (CWE-190): [#def30]
bluez-5.75/attrib/gatt.c:1016:2: known_value_assign: "last" = "65535", its value is now 65535.
bluez-5.75/attrib/gatt.c:1087:2: overflow_const: Expression "dd->start", which is equal to 65536, where "last + 1" is known to be equal to 65536, overflows the type that receives it, an unsigned integer 16 bits wide.
1085|		}
1086|
1087|->		dd->start = last + 1;
1088|
1089|		if (last < dd->end && !uuid_found) {
---
 attrib/gatt.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/attrib/gatt.c b/attrib/gatt.c
index b496dd1ebd95..3cedae9d167a 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -1076,10 +1076,12 @@ static void desc_discovered_cb(guint8 status, const guint8 *ipdu,
 	att_data_list_free(list);
 
 	/*
-	 * If last handle is lower from previous start handle then it is smth
-	 * wrong. Let's stop search, otherwise we might enter infinite loop.
+	 * If last handle is lower from previous start handle or if iterating
+	 * to the next handle from the last possible offset would overflow, then
+	 * something is wrong. Let's stop search, otherwise we might enter
+	 * infinite loop.
 	 */
-	if (last < dd->start) {
+	if (last < dd->start || last == G_MAXUINT16) {
 		err = ATT_ECODE_UNLIKELY;
 		goto done;
 	}
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 03/20] client/gatt: Don't pass negative fd on error
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 01/20] adapter: Use false instead of 0 for bool Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 02/20] attrib/gatt: Guard against possible integer overflow Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 04/20] client/gatt: Check write_value() retval Bastien Nocera
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: NEGATIVE_RETURNS (CWE-394): [#def33]
bluez-5.75/client/gatt.c:973:2: negative_return_fn: Function "io_get_fd(io)" returns a negative number.
bluez-5.75/client/gatt.c:973:2: negative_returns: "io_get_fd(io)" is passed to a parameter that cannot be negative.
971|	msg.msg_iovlen = iovlen;
972|
973|->	ret = sendmsg(io_get_fd(io), &msg, MSG_NOSIGNAL);
974|	if (ret < 0) {
975|		ret = -errno;

Error: NEGATIVE_RETURNS (CWE-394): [#def34]
bluez-5.75/client/gatt.c:1049:2: negative_return_fn: Function "io_get_fd(io)" returns a negative number.
bluez-5.75/client/gatt.c:1049:2: assign: Assigning: "fd" = "io_get_fd(io)".
bluez-5.75/client/gatt.c:1062:2: negative_returns: "fd" is passed to a parameter that cannot be negative.
1060|		msg.msg_iovlen = 1;
1061|
1062|->		bytes_read = recvmsg(fd, &msg, MSG_DONTWAIT);
1063|		if (bytes_read < 0) {
1064|			bt_shell_printf("recvmsg: %s", strerror(errno));
---
 client/gatt.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/client/gatt.c b/client/gatt.c
index 3aaa7a9361b9..6c7603985172 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -966,11 +966,15 @@ static int sock_send(struct io *io, struct iovec *iov, size_t iovlen)
 	struct msghdr msg;
 	int ret;
 
+	ret = io_get_fd(io);
+	if (ret < 0)
+		return ret;
+
 	memset(&msg, 0, sizeof(msg));
 	msg.msg_iov = iov;
 	msg.msg_iovlen = iovlen;
 
-	ret = sendmsg(io_get_fd(io), &msg, MSG_NOSIGNAL);
+	ret = sendmsg(ret, &msg, MSG_NOSIGNAL);
 	if (ret < 0) {
 		ret = -errno;
 		bt_shell_printf("sendmsg: %s", strerror(-ret));
@@ -1052,6 +1056,11 @@ static bool sock_read(struct io *io, void *user_data)
 	if (io != notify_io.io && !chrc)
 		return true;
 
+	if (fd < 0) {
+		bt_shell_printf("recvmsg: %s", strerror(-fd));
+		return false;
+	}
+
 	iov.iov_base = buf;
 	iov.iov_len = sizeof(buf);
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 04/20] client/gatt: Check write_value() retval
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (2 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 03/20] client/gatt: Don't pass negative fd on error Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 05/20] client/main: Fix array access Bastien Nocera
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: CHECKED_RETURN (CWE-252): [#def35]
bluez-5.75/client/gatt.c:3191:3: check_return: Calling "write_value" without checking return value (as is done elsewhere 5 out of 6 times).
bluez-5.75/client/gatt.c:2371:2: example_checked: Example 1: "write_value(&chrc->value_len, &chrc->value, value, value_len, aad->offset, chrc->max_val_len)" has its value checked in "write_value(&chrc->value_len, &chrc->value, value, value_len, aad->offset, chrc->max_val_len)".
bluez-5.75/client/gatt.c:2502:2: example_checked: Example 2: "write_value(&chrc->value_len, &chrc->value, value, value_len, offset, chrc->max_val_len)" has its value checked in "write_value(&chrc->value_len, &chrc->value, value, value_len, offset, chrc->max_val_len)".
bluez-5.75/client/gatt.c:2919:2: example_checked: Example 3: "write_value(&desc->value_len, &desc->value, value, value_len, offset, desc->max_val_len)" has its value checked in "write_value(&desc->value_len, &desc->value, value, value_len, offset, desc->max_val_len)".
bluez-5.75/client/gatt.c:759:3: example_checked: Example 4: "write_value(&c->value_len, &c->value, value, value_len, offset, c->max_val_len)" has its value checked in "write_value(&c->value_len, &c->value, value, value_len, offset, c->max_val_len)".
bluez-5.75/client/gatt.c:775:3: example_checked: Example 5: "write_value(&d->value_len, &d->value, value, value_len, offset, d->max_val_len)" has its value checked in "write_value(&d->value_len, &d->value, value, value_len, offset, d->max_val_len)".
3189|			}
3190|
3191|->			write_value(&chrc->value_len, &chrc->value, value, len,
3192|					0, chrc->max_val_len);
---
 client/gatt.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index 6c7603985172..e85031277bd5 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -3197,9 +3197,13 @@ static void proxy_property_changed(GDBusProxy *proxy, const char *name,
 			dbus_message_iter_get_fixed_array(&array, &value, &len);
 		}
 
-		write_value(&chrc->value_len, &chrc->value, value, len,
-				0, chrc->max_val_len);
-		bt_shell_hexdump(value, len);
+		if (write_value(&chrc->value_len, &chrc->value, value, len,
+				0, chrc->max_val_len)) {
+			bt_shell_printf("Unable to update property value for %s\n",
+					name);
+		} else {
+			bt_shell_hexdump(value, len);
+		}
 	}
 
 	g_dbus_emit_property_changed(conn, chrc->path, CHRC_INTERFACE, name);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 05/20] client/main: Fix array access
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (3 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 04/20] client/gatt: Check write_value() retval Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 06/20] client/main: Fix mismatched free Bastien Nocera
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: CPPCHECK_WARNING (CWE-788): [#def36]
bluez-5.75/client/main.c:833: error[ctuArrayIndex]: Array index out of bounds; 'argv' buffer size is 0 and it is accessed at offset 1.
831|	const char **opt;
832|
833|->	if (!strcmp(argv[1], "help")) {
834|		for (opt = arg_table; opt && *opt; opt++)
835|			bt_shell_printf("%s\n", *opt);
---
 client/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/client/main.c b/client/main.c
index 51d08a67aa1a..f703cc91b24a 100644
--- a/client/main.c
+++ b/client/main.c
@@ -830,6 +830,11 @@ static gboolean parse_argument(int argc, char *argv[], const char **arg_table,
 {
 	const char **opt;
 
+	if (argc < 2) {
+		bt_shell_printf("Missing argument to %s\n", argv[0]);
+		return FALSE;
+	}
+
 	if (!strcmp(argv[1], "help")) {
 		for (opt = arg_table; opt && *opt; opt++)
 			bt_shell_printf("%s\n", *opt);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 06/20] client/main: Fix mismatched free
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (4 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 05/20] client/main: Fix array access Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 07/20] monitor/att: Fix memory leak Bastien Nocera
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: ALLOC_FREE_MISMATCH (CWE-762): [#def37]
bluez-5.75/client/main.c:2108:2: alloc: Allocation of memory which must be freed using "g_free".
bluez-5.75/client/main.c:2108:2: assign: Assigning: "desc" = "g_strdup_printf("\x1b[0;94m[%s]\x1b[0m# ", attr)".
bluez-5.75/client/main.c:2111:2: free: Calling "free" frees "desc" using "free" but it should have been freed using "g_free".
2109|
2110|		bt_shell_set_prompt(desc);
2111|->		free(desc);
2112|   }
2113|
---
 client/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/main.c b/client/main.c
index f703cc91b24a..f967c149e7bd 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2113,7 +2113,7 @@ static void set_default_local_attribute(char *attr)
 	desc = g_strdup_printf(COLOR_BLUE "[%s]" COLOR_OFF "# ", attr);
 
 	bt_shell_set_prompt(desc);
-	free(desc);
+	g_free(desc);
 }
 
 static void cmd_select_attribute(int argc, char *argv[])
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 07/20] monitor/att: Fix memory leak
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (5 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 06/20] client/main: Fix mismatched free Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 08/20] bap: Fix memory leaks Bastien Nocera
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

name2utf8() returns newly allocated memory which needs to be freed.

Error: RESOURCE_LEAK (CWE-772): [#def27] [important]
bluez-5.75/monitor/att.c:2291:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2291:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2293:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2294:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2292|
2293|		print_field("  Media Player Name: %s", name);
2294|-> }
2295|
2296|   static void mp_name_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def28] [important]
bluez-5.75/monitor/att.c:2320:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2320:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2322:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2323:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2321|
2322|		print_field("  Track Title: %s", name);
2323|-> }
2324|
2325|   static void track_title_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def29] [important]
bluez-5.75/monitor/att.c:2453:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2453:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2455:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2456:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2454|
2455|		print_field("  Bearer Name: %s", name);
2456|-> }
2457|
2458|   static void bearer_name_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def30] [important]
bluez-5.75/monitor/att.c:2472:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2472:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2474:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2475:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2473|
2474|		print_field("  Bearer Uci Name: %s", name);
2475|-> }
2476|
2477|   static void print_technology_name(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def31] [important]
bluez-5.75/monitor/att.c:2541:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2541:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2543:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2544:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2542|
2543|		print_field("  Uri scheme Name: %s", name);
2544|-> }
2545|
2546|   static void bearer_uri_schemes_list_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def32] [important]
bluez-5.75/monitor/att.c:2653:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2653:2: var_assign: Assigning: "call_uri" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2655:2: noescape: Resource "call_uri" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2660:1: leaked_storage: Variable "call_uri" going out of scope leaks the storage it points to.
2658|		if (frame->size)
2659|			print_hex_field("  call_list Data", frame->data, frame->size);
2660|-> }
2661|
2662|   static void bearer_current_call_list_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def33] [important]
bluez-5.75/monitor/att.c:2741:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2741:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2743:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2748:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2746|		if (frame->size)
2747|			print_hex_field("  Data", frame->data, frame->size);
2748|-> }
2749|
2750|   static void incom_target_bearer_uri_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def34] [important]
bluez-5.75/monitor/att.c:2851:3: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:2851:3: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:2852:3: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:2871:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
2869|		if (frame->size)
2870|			print_hex_field("call_cp Data", frame->data, frame->size);
2871|-> }
2872|
2873|   static void print_call_cp_notification(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def35] [important]
bluez-5.75/monitor/att.c:3046:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:3046:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:3048:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:3053:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
3051|		if (frame->size)
3052|			print_hex_field(" Data", frame->data, frame->size);
3053|-> }
3054|
3055|   static void incoming_call_read(const struct l2cap_frame *frame)

Error: RESOURCE_LEAK (CWE-772): [#def36] [important]
bluez-5.75/monitor/att.c:3077:2: alloc_fn: Storage is returned from allocation function "name2utf8".
bluez-5.75/monitor/att.c:3077:2: var_assign: Assigning: "name" = storage returned from "name2utf8((uint8_t *)frame->data, frame->size)".
bluez-5.75/monitor/att.c:3079:2: noescape: Resource "name" is not freed or pointed-to in "printf". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/monitor/att.c:3084:1: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
3082|		if (frame->size)
3083|			print_hex_field(" Data", frame->data, frame->size);
3084|-> }
3085|
3086|   static void call_friendly_name_read(const struct l2cap_frame *frame)
---
 monitor/att.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/monitor/att.c b/monitor/att.c
index b3fb3ba6a0ad..a23347ef7ede 100644
--- a/monitor/att.c
+++ b/monitor/att.c
@@ -2291,6 +2291,8 @@ static void print_mp_name(const struct l2cap_frame *frame)
 	name = name2utf8((uint8_t *)frame->data, frame->size);
 
 	print_field("  Media Player Name: %s", name);
+
+	g_free(name);
 }
 
 static void mp_name_read(const struct l2cap_frame *frame)
@@ -2320,6 +2322,8 @@ static void print_track_title(const struct l2cap_frame *frame)
 	name = name2utf8((uint8_t *)frame->data, frame->size);
 
 	print_field("  Track Title: %s", name);
+
+	g_free(name);
 }
 
 static void track_title_read(const struct l2cap_frame *frame)
@@ -2453,6 +2457,8 @@ static void print_bearer_name(const struct l2cap_frame *frame)
 	name = name2utf8((uint8_t *)frame->data, frame->size);
 
 	print_field("  Bearer Name: %s", name);
+
+	g_free(name);
 }
 
 static void bearer_name_read(const struct l2cap_frame *frame)
@@ -2472,6 +2478,8 @@ static void bearer_uci_read(const struct l2cap_frame *frame)
 	name = name2utf8((uint8_t *)frame->data, frame->size);
 
 	print_field("  Bearer Uci Name: %s", name);
+
+	g_free(name);
 }
 
 static void print_technology_name(const struct l2cap_frame *frame)
@@ -2541,6 +2549,8 @@ static void print_uri_scheme_list(const struct l2cap_frame *frame)
 	name = name2utf8((uint8_t *)frame->data, frame->size);
 
 	print_field("  Uri scheme Name: %s", name);
+
+	g_free(name);
 }
 
 static void bearer_uri_schemes_list_read(const struct l2cap_frame *frame)
@@ -2654,6 +2664,8 @@ static void print_call_list(const struct l2cap_frame *frame)
 
 	print_field("  call_uri: %s", call_uri);
 
+	g_free(call_uri);
+
 done:
 	if (frame->size)
 		print_hex_field("  call_list Data", frame->data, frame->size);
@@ -2742,6 +2754,8 @@ static void print_target_uri(const struct l2cap_frame *frame)
 
 	print_field("  Uri: %s", name);
 
+	g_free(name);
+
 done:
 	if (frame->size)
 		print_hex_field("  Data", frame->data, frame->size);
@@ -2850,6 +2864,7 @@ static void print_call_cp(const struct l2cap_frame *frame)
 		str = "Originate";
 		name = name2utf8((uint8_t *)frame->data, frame->size);
 		print_field("  Operation: %s  Uri: %s", str, name);
+		g_free(name);
 		break;
 	case 0x05:
 		str = "Join";
@@ -3047,6 +3062,8 @@ static void print_incom_call(const struct l2cap_frame *frame)
 
 	print_field("  call_string: %s", name);
 
+	g_free(name);
+
 done:
 	if (frame->size)
 		print_hex_field(" Data", frame->data, frame->size);
@@ -3078,6 +3095,8 @@ static void print_call_friendly_name(const struct l2cap_frame *frame)
 
 	print_field("  Friendly Name: %s", name);
 
+	g_free(name);
+
 done:
 	if (frame->size)
 		print_hex_field(" Data", frame->data, frame->size);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 08/20] bap: Fix memory leaks
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (6 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 07/20] monitor/att: Fix memory leak Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 09/20] media: Fix memory leak Bastien Nocera
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def37] [important]
bluez-5.75/profiles/audio/bap.c:1064:13: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1064:13: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1064:13: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1064:13: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1064:3: var_assign: Assigning: "l2_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1066:4: leaked_storage: Variable "l2_caps" going out of scope leaks the storage it points to.
1064|			l2_caps = new0(struct iovec, 1);
1065|			if (!util_iov_pull_u8(&iov, (void *)&l2_caps->iov_len))
1066|->				goto fail;
1067|
1068|			util_iov_memcpy(l2_caps, util_iov_pull_mem(&iov,

Error: RESOURCE_LEAK (CWE-772): [#def38] [important]
bluez-5.75/profiles/audio/bap.c:1064:13: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1064:13: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1064:13: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1064:13: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1064:3: var_assign: Assigning: "l2_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1068:3: noescape: Resource "l2_caps" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1080:4: leaked_storage: Variable "l2_caps" going out of scope leaks the storage it points to.
1078|			meta = new0(struct iovec, 1);
1079|			if (!util_iov_pull_u8(&iov, (void *)&meta->iov_len))
1080|->				goto fail;
1081|
1082|			util_iov_memcpy(meta,

Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
bluez-5.75/profiles/audio/bap.c:1078:10: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1078:10: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1078:10: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1078:10: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1078:3: var_assign: Assigning: "meta" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1080:4: leaked_storage: Variable "meta" going out of scope leaks the storage it points to.
1078|			meta = new0(struct iovec, 1);
1079|			if (!util_iov_pull_u8(&iov, (void *)&meta->iov_len))
1080|->				goto fail;
1081|
1082|			util_iov_memcpy(meta,

Error: RESOURCE_LEAK (CWE-772): [#def40] [important]
bluez-5.75/profiles/audio/bap.c:1064:13: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1064:13: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1064:13: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1064:13: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1064:3: var_assign: Assigning: "l2_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1068:3: noescape: Resource "l2_caps" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "l2_caps" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "l2_caps" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1097:5: leaked_storage: Variable "l2_caps" going out of scope leaks the storage it points to.
1095|
1096|				if (!util_iov_pull_u8(&iov, &bis_index))
1097|->					goto fail;
1098|
1099|				util_debug(func, NULL, "BIS #%d", bis_index);

Error: RESOURCE_LEAK (CWE-772): [#def41] [important]
bluez-5.75/profiles/audio/bap.c:1078:10: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1078:10: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1078:10: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1078:10: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1078:3: var_assign: Assigning: "meta" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1082:3: noescape: Resource "meta" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "meta" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "meta" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1097:5: leaked_storage: Variable "meta" going out of scope leaks the storage it points to.
1095|
1096|				if (!util_iov_pull_u8(&iov, &bis_index))
1097|->					goto fail;
1098|
1099|				util_debug(func, NULL, "BIS #%d", bis_index);

Error: RESOURCE_LEAK (CWE-772): [#def42] [important]
bluez-5.75/profiles/audio/bap.c:1064:13: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1064:13: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1064:13: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1064:13: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1064:3: var_assign: Assigning: "l2_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1068:3: noescape: Resource "l2_caps" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1104:5: leaked_storage: Variable "l2_caps" going out of scope leaks the storage it points to.
1102|				l3_caps = new0(struct iovec, 1);
1103|				if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len))
1104|->					goto fail;
1105|
1106|				util_iov_memcpy(l3_caps,

Error: RESOURCE_LEAK (CWE-772): [#def43] [important]
bluez-5.75/profiles/audio/bap.c:1102:14: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1102:14: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1102:14: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1102:14: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1102:4: var_assign: Assigning: "l3_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1104:5: leaked_storage: Variable "l3_caps" going out of scope leaks the storage it points to.
1102|				l3_caps = new0(struct iovec, 1);
1103|				if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len))
1104|->					goto fail;
1105|
1106|				util_iov_memcpy(l3_caps,

Error: RESOURCE_LEAK (CWE-772): [#def44] [important]
bluez-5.75/profiles/audio/bap.c:1078:10: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1078:10: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1078:10: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1078:10: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1078:3: var_assign: Assigning: "meta" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1082:3: noescape: Resource "meta" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1104:5: leaked_storage: Variable "meta" going out of scope leaks the storage it points to.
1102|				l3_caps = new0(struct iovec, 1);
1103|				if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len))
1104|->					goto fail;
1105|
1106|				util_iov_memcpy(l3_caps,

Error: RESOURCE_LEAK (CWE-772): [#def45] [important]
bluez-5.75/profiles/audio/bap.c:1064:13: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1064:13: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1064:13: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1064:13: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1064:3: var_assign: Assigning: "l2_caps" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1068:3: noescape: Resource "l2_caps" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "l2_caps" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "l2_caps" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1123:2: leaked_storage: Variable "l2_caps" going out of scope leaks the storage it points to.
1121|			}
1122|
1123|->		}
1124|		return true;
1125|

Error: RESOURCE_LEAK (CWE-772): [#def46] [important]
bluez-5.75/profiles/audio/bap.c:1078:10: alloc_fn: Storage is returned from allocation function "util_malloc".
bluez-5.75/profiles/audio/bap.c:1078:10: var_assign: Assigning: "__p" = storage returned from "util_malloc(__n * __s)".
bluez-5.75/profiles/audio/bap.c:1078:10: noescape: Resource "__p" is not freed or pointed-to in "memset". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/bap.c:1078:10: leaked_storage: Variable "__p" going out of scope leaks the storage it points to.
bluez-5.75/profiles/audio/bap.c:1078:3: var_assign: Assigning: "meta" = "({...; __p;})".
bluez-5.75/profiles/audio/bap.c:1082:3: noescape: Resource "meta" is not freed or pointed-to in "util_iov_memcpy".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "meta" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1119:4: noescape: Resource "meta" is not freed or pointed-to in "bt_bap_add_bis".
bluez-5.75/profiles/audio/bap.c:1123:2: leaked_storage: Variable "meta" going out of scope leaks the storage it points to.
1121|			}
1122|
1123|->		}
1124|		return true;
1125|
---
 profiles/audio/bap.c | 47 +++++++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/profiles/audio/bap.c b/profiles/audio/bap.c
index 8e4f4b311fba..15024e26f843 100644
--- a/profiles/audio/bap.c
+++ b/profiles/audio/bap.c
@@ -1028,6 +1028,7 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 	};
 	uint32_t pres_delay;
 	uint8_t num_subgroups;
+	bool ret = true;
 
 	util_debug(func, NULL, "BASE len: %ld", iov.iov_len);
 
@@ -1043,13 +1044,15 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 	for (int idx = 0; idx < num_subgroups; idx++) {
 		uint8_t num_bis;
 		struct bt_bap_codec codec;
-		struct iovec *l2_caps;
-		struct iovec *meta;
+		struct iovec *l2_caps = NULL;
+		struct iovec *meta = NULL;
 
 		util_debug(func, NULL, "Subgroup #%d", idx);
 
-		if (!util_iov_pull_u8(&iov, &num_bis))
+		if (!util_iov_pull_u8(&iov, &num_bis)) {
+			ret = false;
 			goto fail;
+		}
 		util_debug(func, NULL, "Number of BISes: %d", num_bis);
 
 		memcpy(&codec,
@@ -1062,8 +1065,10 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 		/* Level 2 */
 		/* Read Codec Specific Configuration */
 		l2_caps = new0(struct iovec, 1);
-		if (!util_iov_pull_u8(&iov, (void *)&l2_caps->iov_len))
-			goto fail;
+		if (!util_iov_pull_u8(&iov, (void *)&l2_caps->iov_len)) {
+			ret = false;
+			goto group_fail;
+		}
 
 		util_iov_memcpy(l2_caps, util_iov_pull_mem(&iov,
 				l2_caps->iov_len),
@@ -1076,8 +1081,10 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 
 		/* Read Metadata */
 		meta = new0(struct iovec, 1);
-		if (!util_iov_pull_u8(&iov, (void *)&meta->iov_len))
-			goto fail;
+		if (!util_iov_pull_u8(&iov, (void *)&meta->iov_len)) {
+			ret = false;
+			goto group_fail;
+		}
 
 		util_iov_memcpy(meta,
 				util_iov_pull_mem(&iov, meta->iov_len),
@@ -1093,15 +1100,20 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 			uint8_t bis_index;
 			struct iovec *l3_caps;
 
-			if (!util_iov_pull_u8(&iov, &bis_index))
-				goto fail;
+			if (!util_iov_pull_u8(&iov, &bis_index)) {
+				ret = false;
+				goto group_fail;
+			}
 
 			util_debug(func, NULL, "BIS #%d", bis_index);
 
 			/* Read Codec Specific Configuration */
 			l3_caps = new0(struct iovec, 1);
-			if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len))
-				goto fail;
+			if (!util_iov_pull_u8(&iov, (void *)&l3_caps->iov_len)) {
+				free(l3_caps);
+				ret = false;
+				goto group_fail;
+			}
 
 			util_iov_memcpy(l3_caps,
 					util_iov_pull_mem(&iov,
@@ -1120,13 +1132,20 @@ static bool parse_base(struct bt_bap *bap, struct bt_iso_base *base,
 					meta);
 		}
 
+group_fail:
+		if (l2_caps != NULL)
+			free(l2_caps);
+		if (meta != NULL)
+			free(meta);
+		if (!ret)
+			break;
 	}
-	return true;
 
 fail:
-	util_debug(func, NULL, "Unable to parse Base");
+	if (!ret)
+		util_debug(func, NULL, "Unable to parse Base");
 
-	return false;
+	return ret;
 }
 
 static void iso_pa_sync_confirm_cb(GIOChannel *io, void *user_data)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 09/20] media: Fix memory leak
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (7 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 08/20] bap: Fix memory leaks Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 10/20] main: Fix memory leaks Bastien Nocera
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def47] [important]
bluez-5.75/profiles/audio/media.c:1278:2: alloc_arg: "asprintf" allocates memory that is stored into "name". [Note: The source code implementation of the function has been overridden by a builtin model.]
bluez-5.75/profiles/audio/media.c:1291:2: noescape: Resource "name" is not freed or pointed-to in "bt_bap_add_vendor_pac".
bluez-5.75/profiles/audio/media.c:1297:3: leaked_storage: Variable "name" going out of scope leaks the storage it points to.
1295|			error("Unable to create PAC");
1296|			free(metadata);
1297|->			return false;
1298|		}
1299|
---
 profiles/audio/media.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 07147a25d532..4bbd584deaba 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1293,6 +1293,7 @@ static bool endpoint_init_pac(struct media_endpoint *endpoint, uint8_t type,
 				&data, metadata);
 	if (!endpoint->pac) {
 		error("Unable to create PAC");
+		free(name);
 		free(metadata);
 		return false;
 	}
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 10/20] main: Fix memory leaks
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (8 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 09/20] media: Fix memory leak Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 11/20] isotest: Consider "0" fd to be valid Bastien Nocera
                   ` (10 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def51] [important]
bluez-5.75/src/main.c:451:2: alloc_arg: "parse_config_string" allocates memory that is stored into "str".
bluez-5.75/src/main.c:454:2: identity_transfer: Passing "str" as argument 1 to function "strtol", which sets "endptr" to that argument.
bluez-5.75/src/main.c:456:3: noescape: Assuming resource "str" is not freed or pointed-to as ellipsis argument to "btd_error".
bluez-5.75/src/main.c:457:3: leaked_storage: Variable "endptr" going out of scope leaks the storage it points to.
bluez-5.75/src/main.c:457:3: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
455|	if (!endptr || *endptr != '\0') {
456|		error("%s.%s = %s is not integer", group, key, str);
457|->		return false;
458|	}
459|

Error: RESOURCE_LEAK (CWE-772): [#def52] [important]
bluez-5.75/src/main.c:451:2: alloc_arg: "parse_config_string" allocates memory that is stored into "str".
bluez-5.75/src/main.c:454:2: identity_transfer: Passing "str" as argument 1 to function "strtol", which sets "endptr" to that argument.
bluez-5.75/src/main.c:463:3: leaked_storage: Variable "endptr" going out of scope leaks the storage it points to.
bluez-5.75/src/main.c:463:3: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
461|		warn("%s.%s = %zu is out of range (< %zu)", group, key, tmp,
462|									min);
463|->		return false;
464|	}
465|

Error: RESOURCE_LEAK (CWE-772): [#def53] [important]
bluez-5.75/src/main.c:451:2: alloc_arg: "parse_config_string" allocates memory that is stored into "str".
bluez-5.75/src/main.c:454:2: identity_transfer: Passing "str" as argument 1 to function "strtol", which sets "endptr" to that argument.
bluez-5.75/src/main.c:475:2: leaked_storage: Variable "endptr" going out of scope leaks the storage it points to.
bluez-5.75/src/main.c:475:2: leaked_storage: Variable "str" going out of scope leaks the storage it points to.
473|		*val = tmp;
474|
475|->	return true;
476|   }
477|
---
 src/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/main.c b/src/main.c
index 23af6781d931..ac840d684f6d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -454,21 +454,25 @@ static bool parse_config_int(GKeyFile *config, const char *group,
 	tmp = strtol(str, &endptr, 0);
 	if (!endptr || *endptr != '\0') {
 		error("%s.%s = %s is not integer", group, key, str);
+		g_free(str);
 		return false;
 	}
 
 	if (tmp < min) {
+		g_free(str);
 		warn("%s.%s = %zu is out of range (< %zu)", group, key, tmp,
 									min);
 		return false;
 	}
 
 	if (tmp > max) {
+		g_free(str);
 		warn("%s.%s = %zu is out of range (> %zu)", group, key, tmp,
 									max);
 		return false;
 	}
 
+	g_free(str);
 	if (val)
 		*val = tmp;
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 11/20] isotest: Consider "0" fd to be valid
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (9 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 10/20] main: Fix memory leaks Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 12/20] isotest: Fix error check after opening file Bastien Nocera
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def79] [important]
bluez-5.75/tools/isotest.c:923:4: open_fn: Returning handle opened by "open_file".
bluez-5.75/tools/isotest.c:923:4: var_assign: Assigning: "fd" = handle returned from "open_file(altername)".
bluez-5.75/tools/isotest.c:925:3: off_by_one: Testing whether handle "fd" is strictly greater than zero is suspicious.  "fd" leaks when it is zero.
bluez-5.75/tools/isotest.c:925:3: remediation: Did you intend to include equality with zero?
bluez-5.75/tools/isotest.c:926:4: overwrite_var: Overwriting handle "fd" in "fd = open_file(filename)" leaks the handle.
924|
925|		if (fd <= 0)
926|->			fd = open_file(filename);
927|	}
928|
---
 tools/isotest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/isotest.c b/tools/isotest.c
index 7e875fa58b15..810d15d2df2a 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -922,7 +922,7 @@ static void send_mode(char *filename, char *peer, int i, bool repeat)
 		if (!err)
 			fd = open_file(altername);
 
-		if (fd <= 0)
+		if (fd < 0)
 			fd = open_file(filename);
 	}
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 12/20] isotest: Fix error check after opening file
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (10 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 11/20] isotest: Consider "0" fd to be valid Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 13/20] client/player: Fix copy/paste error Bastien Nocera
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Consider "0" to be a valid fd.
---
 tools/isotest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/isotest.c b/tools/isotest.c
index 810d15d2df2a..ddace0da3044 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -720,7 +720,7 @@ static int open_file(const char *filename)
 	syslog(LOG_INFO, "Opening %s ...", filename);
 
 	fd = open(filename, O_RDONLY);
-	if (fd <= 0) {
+	if (fd < 0) {
 		syslog(LOG_ERR, "Can't open file %s: %s\n",
 						filename, strerror(errno));
 	}
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 13/20] client/player: Fix copy/paste error
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (11 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 12/20] isotest: Fix error check after opening file Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 14/20] shared/vcp: " Bastien Nocera
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: COPY_PASTE_ERROR (CWE-398): [#def95] [important]
bluez-5.75/client/player.c:1846:6: original: "qos->sync_cte_type" looks like the original copy.
bluez-5.75/client/player.c:1852:6: copy_paste_error: "sync_cte_type" in "qos->sync_cte_type" looks like a copy-paste error.
bluez-5.75/client/player.c:1852:6: remediation: Should it say "mse" instead?
1850|		}
1851|
1852|->		if (qos->sync_cte_type) {
1853|			bt_shell_printf("MSE %u\n", qos->mse);
1854|			g_dbus_dict_append_entry(iter, "MSE", DBUS_TYPE_BYTE,
---
 client/player.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client/player.c b/client/player.c
index 6b70e9ed3f9d..7f67425aaf8f 100644
--- a/client/player.c
+++ b/client/player.c
@@ -1849,7 +1849,7 @@ static void append_bcast_qos(DBusMessageIter *iter, struct endpoint_config *cfg)
 					&qos->sync_cte_type);
 	}
 
-	if (qos->sync_cte_type) {
+	if (qos->mse) {
 		bt_shell_printf("MSE %u\n", qos->mse);
 		g_dbus_dict_append_entry(iter, "MSE", DBUS_TYPE_BYTE,
 						&qos->mse);
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 14/20] shared/vcp: Fix copy/paste error
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (12 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 13/20] client/player: Fix copy/paste error Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 15/20] isotest: Fix fd leak Bastien Nocera
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: COPY_PASTE_ERROR (CWE-398): [#def97] [important]
bluez-5.75/src/shared/vcp.c:2610:16: original: "aics->gain_stting_prop" looks like the original copy.
bluez-5.75/src/shared/vcp.c:2625:16: copy_paste_error: "gain_stting_prop" in "aics->gain_stting_prop" looks like a copy-paste error.
bluez-5.75/src/shared/vcp.c:2625:16: remediation: Should it say "aud_ip_type" instead?
2623|
2624|			aics = vcp_get_aics(vcp);
2625|->			if (!aics || aics->gain_stting_prop)
2626|				return;
2627|
---
 src/shared/vcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index 7ba54e64adc0..b7e17e448b74 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -2622,7 +2622,7 @@ static void foreach_aics_char(struct gatt_db_attribute *attr, void *user_data)
 			value_handle);
 
 		aics = vcp_get_aics(vcp);
-		if (!aics || aics->gain_stting_prop)
+		if (!aics || aics->aud_ip_type)
 			return;
 
 		aics->aud_ip_type = attr;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 15/20] isotest: Fix fd leak
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (13 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 14/20] shared/vcp: " Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 16/20] iso-tester: " Bastien Nocera
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def65] [important]
bluez-5.75/tools/isotest.c:923:4: open_fn: Returning handle opened by "open_file".
bluez-5.75/tools/isotest.c:923:4: var_assign: Assigning: "fd" = handle returned from "open_file(altername)".
bluez-5.75/tools/isotest.c:953:3: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
951|
952|		free(sk_arr);
953|->		return;
954|	}
955|
---
 tools/isotest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/isotest.c b/tools/isotest.c
index ddace0da3044..58293133a304 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -950,6 +950,8 @@ static void send_mode(char *filename, char *peer, int i, bool repeat)
 			close(sk_arr[i]);
 
 		free(sk_arr);
+		if (fd >= 0)
+			close(fd);
 		return;
 	}
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 16/20] iso-tester: Fix fd leak
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (14 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 15/20] isotest: Fix fd leak Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 17/20] sdp: Fix use of uninitialised memory Bastien Nocera
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def63] [important]
bluez-5.75/tools/iso-tester.c:1796:2: open_fn: Returning handle opened by "socket".
bluez-5.75/tools/iso-tester.c:1796:2: var_assign: Assigning: "sk" = handle returned from "socket(31, 2053, 8)".
bluez-5.75/tools/iso-tester.c:1807:3: leaked_handle: Handle variable "sk" going out of scope leaks the handle.
1805|		if (!master_bdaddr) {
1806|			tester_warn("No master bdaddr");
1807|->			return -ENODEV;
1808|		}
1809|
---
 tools/iso-tester.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/iso-tester.c b/tools/iso-tester.c
index 046606068206..d54fa56ecd44 100644
--- a/tools/iso-tester.c
+++ b/tools/iso-tester.c
@@ -1804,6 +1804,7 @@ static int create_iso_sock(struct test_data *data)
 	master_bdaddr = hciemu_get_central_bdaddr(data->hciemu);
 	if (!master_bdaddr) {
 		tester_warn("No master bdaddr");
+		close(sk);
 		return -ENODEV;
 	}
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 17/20] sdp: Fix use of uninitialised memory
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (15 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 16/20] iso-tester: " Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 18/20] monitor: Work-around memory leak warning Bastien Nocera
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: UNINIT (CWE-457): [#def10] [important]
bluez-5.75/lib/sdp.c:2302:2: alloc_fn: Calling "malloc" which returns uninitialized memory.
bluez-5.75/lib/sdp.c:2302:2: assign: Assigning: "seqDTDs" = "malloc(seqlen * 8UL)", which points to uninitialized data.
bluez-5.75/lib/sdp.c:2355:2: uninit_use_in_call: Using uninitialized value "*seqDTDs" when calling "sdp_seq_alloc".
2353|			}
2354|		}
2355|->		seq = sdp_seq_alloc(seqDTDs, seqs, seqlen);
2356|		free(seqDTDs);
2357|		free(seqs);
---
 lib/sdp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 34b0dbb94eb0..d43bbbd2de05 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2299,7 +2299,7 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
 	sdp_list_t *p;
 
 	seqlen = sdp_list_len(proto);
-	seqDTDs = malloc(seqlen * sizeof(void *));
+	seqDTDs = bt_malloc0(seqlen * sizeof(void *));
 	if (!seqDTDs)
 		return NULL;
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 18/20] monitor: Work-around memory leak warning
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (16 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 17/20] sdp: Fix use of uninitialised memory Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 19/20] avrcp: Fix uninitialised memory usage Bastien Nocera
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Work-around this warning by making the so pointer global.

Error: RESOURCE_LEAK (CWE-772): [#def29] [important]
bluez-5.75/monitor/jlink.c:87:3: alloc_fn: Storage is returned from allocation function "dlopen".
bluez-5.75/monitor/jlink.c:87:3: var_assign: Assigning: "so" = storage returned from "dlopen(jlink_so_name[i], 1)".
bluez-5.75/monitor/jlink.c:95:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:96:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:97:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:98:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:99:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:100:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:101:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:102:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:103:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:104:2: noescape: Resource "so" is not freed or pointed-to in "dlsym".
bluez-5.75/monitor/jlink.c:116:2: leaked_storage: Variable "so" going out of scope leaks the storage it points to.
114|
115|	/* don't dlclose(so) here cause symbols from it are in use now */
116|->	return 0;
117|   }
118|
---
 monitor/jlink.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/monitor/jlink.c b/monitor/jlink.c
index f9d4037f4cdf..e08cc87139c9 100644
--- a/monitor/jlink.c
+++ b/monitor/jlink.c
@@ -47,6 +47,7 @@ struct rtt_desc {
 };
 
 static struct rtt_desc rtt_desc;
+static void *so = NULL;
 
 typedef int (*jlink_emu_selectbyusbsn_func) (unsigned int sn);
 typedef int (*jlink_open_func) (void);
@@ -80,7 +81,6 @@ static struct jlink jlink;
 
 int jlink_init(void)
 {
-	void *so;
 	unsigned int i;
 
 	for (i = 0; i < NELEM(jlink_so_name); i++) {
@@ -109,6 +109,7 @@ int jlink_init(void)
 			!jlink.emu_getproductname ||
 			!jlink.rtterminal_control || !jlink.rtterminal_read) {
 		dlclose(so);
+		so = NULL;
 		return -EIO;
 	}
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 19/20] avrcp: Fix uninitialised memory usage
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (17 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 18/20] monitor: Work-around memory leak warning Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 12:10 ` [BlueZ v2 20/20] main: Simplify variable assignment Bastien Nocera
  2024-05-10 15:40 ` [BlueZ v2 00/20] Fix a number of static analysis issues patchwork-bot+bluetooth
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: UNINIT (CWE-457): [#def35] [important]
bluez-5.75/profiles/audio/avrcp.c:2550:2: var_decl: Declaring variable "name" without initializer.
bluez-5.75/profiles/audio/avrcp.c:2567:2: uninit_use_in_call: Using uninitialized value "*name" when calling "media_player_create_item".
2565|		mp = player->user_data;
2566|
2567|->		item = media_player_create_item(mp, name, PLAYER_ITEM_TYPE_AUDIO, uid);
2568|		if (item == NULL)
2569|			return NULL;

Error: UNINIT (CWE-457): [#def36] [important]
bluez-5.75/profiles/audio/avrcp.c:2583:2: var_decl: Declaring variable "name" without initializer.
bluez-5.75/profiles/audio/avrcp.c:2601:2: uninit_use_in_call: Using uninitialized value "*name" when calling "media_player_create_folder".
2599|		}
2600|
2601|->		item = media_player_create_folder(mp, name, type, uid);
2602|		if (!item)
2603|			return NULL;
---
 profiles/audio/avrcp.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 36ce01a14eea..752e55be37a4 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2555,11 +2555,10 @@ static struct media_item *parse_media_element(struct avrcp *session,
 
 	uid = get_be64(&operands[0]);
 
+	memset(name, 0, sizeof(name));
 	namelen = MIN(get_be16(&operands[11]), sizeof(name) - 1);
-	if (namelen > 0) {
+	if (namelen > 0)
 		memcpy(name, &operands[13], namelen);
-		name[namelen] = '\0';
-	}
 
 	player = session->controller->player;
 	mp = player->user_data;
@@ -2592,11 +2591,10 @@ static struct media_item *parse_media_folder(struct avrcp *session,
 	type = operands[8];
 	playable = operands[9];
 
+	memset(name, 0, sizeof(name));
 	namelen = MIN(get_be16(&operands[12]), sizeof(name) - 1);
-	if (namelen > 0) {
+	if (namelen > 0)
 		memcpy(name, &operands[14], namelen);
-		name[namelen] = '\0';
-	}
 
 	item = media_player_create_folder(mp, name, type, uid);
 	if (!item)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [BlueZ v2 20/20] main: Simplify variable assignment
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (18 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 19/20] avrcp: Fix uninitialised memory usage Bastien Nocera
@ 2024-05-10 12:10 ` Bastien Nocera
  2024-05-10 15:40 ` [BlueZ v2 00/20] Fix a number of static analysis issues patchwork-bot+bluetooth
  20 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-10 12:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
bluez-5.75/src/main.c:425:2: alloc_fn: Storage is returned from allocation function "g_key_file_get_string".
bluez-5.75/src/main.c:425:2: var_assign: Assigning: "tmp" = storage returned from "g_key_file_get_string(config, group, key, &err)".
bluez-5.75/src/main.c:433:2: noescape: Assuming resource "tmp" is not freed or pointed-to as ellipsis argument to "btd_debug".
bluez-5.75/src/main.c:440:2: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to.
438|	}
439|
440|->	return true;
441|   }
442|
---
 src/main.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/main.c b/src/main.c
index ac840d684f6d..a31740179941 100644
--- a/src/main.c
+++ b/src/main.c
@@ -420,9 +420,10 @@ static bool parse_config_string(GKeyFile *config, const char *group,
 					const char *key, char **val)
 {
 	GError *err = NULL;
-	char *tmp;
 
-	tmp = g_key_file_get_string(config, group, key, &err);
+	g_return_val_if_fail(val, false);
+
+	*val = g_key_file_get_string(config, group, key, &err);
 	if (err) {
 		if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
 			DBG("%s", err->message);
@@ -430,12 +431,7 @@ static bool parse_config_string(GKeyFile *config, const char *group,
 		return false;
 	}
 
-	DBG("%s.%s = %s", group, key, tmp);
-
-	if (val) {
-		g_free(*val);
-		*val = tmp;
-	}
+	DBG("%s.%s = %s", group, key, *val);
 
 	return true;
 }
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [BlueZ v2 00/20] Fix a number of static analysis issues
  2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
                   ` (19 preceding siblings ...)
  2024-05-10 12:10 ` [BlueZ v2 20/20] main: Simplify variable assignment Bastien Nocera
@ 2024-05-10 15:40 ` patchwork-bot+bluetooth
  2024-05-10 16:42   ` Luiz Augusto von Dentz
  20 siblings, 1 reply; 24+ messages in thread
From: patchwork-bot+bluetooth @ 2024-05-10 15:40 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-bluetooth

Hello:

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

On Fri, 10 May 2024 14:10:10 +0200 you wrote:
> Changes since v1:
> - added 6 patches
> - Fix syntax error in "client/gatt: Check write_value() retval"
> 
> Bastien Nocera (20):
>   adapter: Use false instead of 0 for bool
>   attrib/gatt: Guard against possible integer overflow
>   client/gatt: Don't pass negative fd on error
>   client/gatt: Check write_value() retval
>   client/main: Fix array access
>   client/main: Fix mismatched free
>   monitor/att: Fix memory leak
>   bap: Fix memory leaks
>   media: Fix memory leak
>   main: Fix memory leaks
>   isotest: Consider "0" fd to be valid
>   isotest: Fix error check after opening file
>   client/player: Fix copy/paste error
>   shared/vcp: Fix copy/paste error
>   isotest: Fix fd leak
>   iso-tester: Fix fd leak
>   sdp: Fix use of uninitialised memory
>   monitor: Work-around memory leak warning
>   avrcp: Fix uninitialised memory usage
>   main: Simplify variable assignment
> 
> [...]

Here is the summary with links:
  - [BlueZ,v2,01/20] adapter: Use false instead of 0 for bool
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d3fcc77f99da
  - [BlueZ,v2,02/20] attrib/gatt: Guard against possible integer overflow
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1e22fd9adbb3
  - [BlueZ,v2,03/20] client/gatt: Don't pass negative fd on error
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1ba9e5f21ca2
  - [BlueZ,v2,04/20] client/gatt: Check write_value() retval
    (no matching commit)
  - [BlueZ,v2,05/20] client/main: Fix array access
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f3f762b77b58
  - [BlueZ,v2,06/20] client/main: Fix mismatched free
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ab325450b0c2
  - [BlueZ,v2,07/20] monitor/att: Fix memory leak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0b842fe9b1fe
  - [BlueZ,v2,08/20] bap: Fix memory leaks
    (no matching commit)
  - [BlueZ,v2,09/20] media: Fix memory leak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3652e98d2bb6
  - [BlueZ,v2,10/20] main: Fix memory leaks
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=45d151ec8a0f
  - [BlueZ,v2,11/20] isotest: Consider "0" fd to be valid
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d30dc38b0425
  - [BlueZ,v2,12/20] isotest: Fix error check after opening file
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9a36f191aa78
  - [BlueZ,v2,13/20] client/player: Fix copy/paste error
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6f041df23ecf
  - [BlueZ,v2,14/20] shared/vcp: Fix copy/paste error
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=856353b254da
  - [BlueZ,v2,15/20] isotest: Fix fd leak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3e03788ba80c
  - [BlueZ,v2,16/20] iso-tester: Fix fd leak
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c81f9320357b
  - [BlueZ,v2,17/20] sdp: Fix use of uninitialised memory
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=dc60ce0b460a
  - [BlueZ,v2,18/20] monitor: Work-around memory leak warning
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e5925dbb84fa
  - [BlueZ,v2,19/20] avrcp: Fix uninitialised memory usage
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=af2634ce0a62
  - [BlueZ,v2,20/20] main: Simplify variable assignment
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=87edbabf3956

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



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [BlueZ v2 00/20] Fix a number of static analysis issues
  2024-05-10 15:40 ` [BlueZ v2 00/20] Fix a number of static analysis issues patchwork-bot+bluetooth
@ 2024-05-10 16:42   ` Luiz Augusto von Dentz
  2024-05-14 10:05     ` Bastien Nocera
  0 siblings, 1 reply; 24+ messages in thread
From: Luiz Augusto von Dentz @ 2024-05-10 16:42 UTC (permalink / raw)
  To: patchwork-bot+bluetooth; +Cc: Bastien Nocera, linux-bluetooth

Hi Bastien,

On Fri, May 10, 2024 at 11:48 AM <patchwork-bot+bluetooth@kernel.org> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluez.git (master)
> by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
>
> On Fri, 10 May 2024 14:10:10 +0200 you wrote:
> > Changes since v1:
> > - added 6 patches
> > - Fix syntax error in "client/gatt: Check write_value() retval"
> >
> > Bastien Nocera (20):
> >   adapter: Use false instead of 0 for bool
> >   attrib/gatt: Guard against possible integer overflow
> >   client/gatt: Don't pass negative fd on error
> >   client/gatt: Check write_value() retval
> >   client/main: Fix array access
> >   client/main: Fix mismatched free
> >   monitor/att: Fix memory leak
> >   bap: Fix memory leaks
> >   media: Fix memory leak
> >   main: Fix memory leaks
> >   isotest: Consider "0" fd to be valid
> >   isotest: Fix error check after opening file
> >   client/player: Fix copy/paste error
> >   shared/vcp: Fix copy/paste error
> >   isotest: Fix fd leak
> >   iso-tester: Fix fd leak
> >   sdp: Fix use of uninitialised memory
> >   monitor: Work-around memory leak warning
> >   avrcp: Fix uninitialised memory usage
> >   main: Simplify variable assignment
> >
> > [...]
>
> Here is the summary with links:
>   - [BlueZ,v2,01/20] adapter: Use false instead of 0 for bool
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d3fcc77f99da
>   - [BlueZ,v2,02/20] attrib/gatt: Guard against possible integer overflow
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1e22fd9adbb3
>   - [BlueZ,v2,03/20] client/gatt: Don't pass negative fd on error
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1ba9e5f21ca2
>   - [BlueZ,v2,04/20] client/gatt: Check write_value() retval
>     (no matching commit)
>   - [BlueZ,v2,05/20] client/main: Fix array access
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f3f762b77b58
>   - [BlueZ,v2,06/20] client/main: Fix mismatched free
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ab325450b0c2
>   - [BlueZ,v2,07/20] monitor/att: Fix memory leak
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0b842fe9b1fe
>   - [BlueZ,v2,08/20] bap: Fix memory leaks
>     (no matching commit)
>   - [BlueZ,v2,09/20] media: Fix memory leak
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3652e98d2bb6
>   - [BlueZ,v2,10/20] main: Fix memory leaks
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=45d151ec8a0f
>   - [BlueZ,v2,11/20] isotest: Consider "0" fd to be valid
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d30dc38b0425
>   - [BlueZ,v2,12/20] isotest: Fix error check after opening file
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9a36f191aa78
>   - [BlueZ,v2,13/20] client/player: Fix copy/paste error
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6f041df23ecf
>   - [BlueZ,v2,14/20] shared/vcp: Fix copy/paste error
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=856353b254da
>   - [BlueZ,v2,15/20] isotest: Fix fd leak
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3e03788ba80c
>   - [BlueZ,v2,16/20] iso-tester: Fix fd leak
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c81f9320357b
>   - [BlueZ,v2,17/20] sdp: Fix use of uninitialised memory
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=dc60ce0b460a
>   - [BlueZ,v2,18/20] monitor: Work-around memory leak warning
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e5925dbb84fa
>   - [BlueZ,v2,19/20] avrcp: Fix uninitialised memory usage
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=af2634ce0a62
>   - [BlueZ,v2,20/20] main: Simplify variable assignment
>     https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=87edbabf3956
>
> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html

Had to revert the last one since it was causing bluetoothd to crash at star.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [BlueZ v2 00/20] Fix a number of static analysis issues
  2024-05-10 16:42   ` Luiz Augusto von Dentz
@ 2024-05-14 10:05     ` Bastien Nocera
  0 siblings, 0 replies; 24+ messages in thread
From: Bastien Nocera @ 2024-05-14 10:05 UTC (permalink / raw)
  To: Luiz Augusto von Dentz, patchwork-bot+bluetooth; +Cc: linux-bluetooth

On Fri, 2024-05-10 at 12:42 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Fri, May 10, 2024 at 11:48 AM <patchwork-bot+bluetooth@kernel.org>
> wrote:
> > 
> > Hello:
> > 
> > This series was applied to bluetooth/bluez.git (master)
> > by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
> > 
> > On Fri, 10 May 2024 14:10:10 +0200 you wrote:
> > > Changes since v1:
> > > - added 6 patches
> > > - Fix syntax error in "client/gatt: Check write_value() retval"
> > > 
> > > Bastien Nocera (20):
> > >   adapter: Use false instead of 0 for bool
> > >   attrib/gatt: Guard against possible integer overflow
> > >   client/gatt: Don't pass negative fd on error
> > >   client/gatt: Check write_value() retval
> > >   client/main: Fix array access
> > >   client/main: Fix mismatched free
> > >   monitor/att: Fix memory leak
> > >   bap: Fix memory leaks
> > >   media: Fix memory leak
> > >   main: Fix memory leaks
> > >   isotest: Consider "0" fd to be valid
> > >   isotest: Fix error check after opening file
> > >   client/player: Fix copy/paste error
> > >   shared/vcp: Fix copy/paste error
> > >   isotest: Fix fd leak
> > >   iso-tester: Fix fd leak
> > >   sdp: Fix use of uninitialised memory
> > >   monitor: Work-around memory leak warning
> > >   avrcp: Fix uninitialised memory usage
> > >   main: Simplify variable assignment
> > > 
> > > [...]
> > 
> > Here is the summary with links:
> >   - [BlueZ,v2,01/20] adapter: Use false instead of 0 for bool
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d3fcc77f99da
> >   - [BlueZ,v2,02/20] attrib/gatt: Guard against possible integer
> > overflow
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1e22fd9adbb3
> >   - [BlueZ,v2,03/20] client/gatt: Don't pass negative fd on error
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1ba9e5f21ca2
> >   - [BlueZ,v2,04/20] client/gatt: Check write_value() retval
> >     (no matching commit)
> >   - [BlueZ,v2,05/20] client/main: Fix array access
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f3f762b77b58
> >   - [BlueZ,v2,06/20] client/main: Fix mismatched free
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ab325450b0c2
> >   - [BlueZ,v2,07/20] monitor/att: Fix memory leak
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=0b842fe9b1fe
> >   - [BlueZ,v2,08/20] bap: Fix memory leaks
> >     (no matching commit)
> >   - [BlueZ,v2,09/20] media: Fix memory leak
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3652e98d2bb6
> >   - [BlueZ,v2,10/20] main: Fix memory leaks
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=45d151ec8a0f
> >   - [BlueZ,v2,11/20] isotest: Consider "0" fd to be valid
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d30dc38b0425
> >   - [BlueZ,v2,12/20] isotest: Fix error check after opening file
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=9a36f191aa78
> >   - [BlueZ,v2,13/20] client/player: Fix copy/paste error
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6f041df23ecf
> >   - [BlueZ,v2,14/20] shared/vcp: Fix copy/paste error
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=856353b254da
> >   - [BlueZ,v2,15/20] isotest: Fix fd leak
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=3e03788ba80c
> >   - [BlueZ,v2,16/20] iso-tester: Fix fd leak
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=c81f9320357b
> >   - [BlueZ,v2,17/20] sdp: Fix use of uninitialised memory
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=dc60ce0b460a
> >   - [BlueZ,v2,18/20] monitor: Work-around memory leak warning
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e5925dbb84fa
> >   - [BlueZ,v2,19/20] avrcp: Fix uninitialised memory usage
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=af2634ce0a62
> >   - [BlueZ,v2,20/20] main: Simplify variable assignment
> >    
> > https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=87edbabf3956
> > 
> > You are awesome, thank you!
> > --
> > Deet-doot-dot, I am a bot.
> > https://korg.docs.kernel.org/patchwork/pwbot.html
> 
> Had to revert the last one since it was causing bluetoothd to crash
> at star.

Thanks very much!

I should have at least mildly tested the resulting builds, my fault, I
also misunderstood what this portion of code did. I've made another
attempt at fixing this issue, and will be submitting more bug fixes in
the near future.

Cheers

> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2024-05-14 10:05 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-10 12:10 [BlueZ v2 00/20] Fix a number of static analysis issues Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 01/20] adapter: Use false instead of 0 for bool Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 02/20] attrib/gatt: Guard against possible integer overflow Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 03/20] client/gatt: Don't pass negative fd on error Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 04/20] client/gatt: Check write_value() retval Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 05/20] client/main: Fix array access Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 06/20] client/main: Fix mismatched free Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 07/20] monitor/att: Fix memory leak Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 08/20] bap: Fix memory leaks Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 09/20] media: Fix memory leak Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 10/20] main: Fix memory leaks Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 11/20] isotest: Consider "0" fd to be valid Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 12/20] isotest: Fix error check after opening file Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 13/20] client/player: Fix copy/paste error Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 14/20] shared/vcp: " Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 15/20] isotest: Fix fd leak Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 16/20] iso-tester: " Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 17/20] sdp: Fix use of uninitialised memory Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 18/20] monitor: Work-around memory leak warning Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 19/20] avrcp: Fix uninitialised memory usage Bastien Nocera
2024-05-10 12:10 ` [BlueZ v2 20/20] main: Simplify variable assignment Bastien Nocera
2024-05-10 15:40 ` [BlueZ v2 00/20] Fix a number of static analysis issues patchwork-bot+bluetooth
2024-05-10 16:42   ` Luiz Augusto von Dentz
2024-05-14 10:05     ` Bastien Nocera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox