All of lore.kernel.org
 help / color / mirror / Atom feed
* [Bluez 00/13] Fixing memory leak, leaked_handle and use_after
@ 2022-05-31  7:41 Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 01/13] Fixing memory leak issue in gatt.c Gopal Tiwari
                   ` (13 more replies)
  0 siblings, 14 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Following fixes are reported by coverity tool.  

Gopal Tiwari (13):
  Fixing memory leak issue in gatt.c
  Fixing memory leakage in appkey.c
  Fixing memory leak in jlink.c
  Fixing memory leak in sixaxis.c
  Fixing leaked_handle in cltest.c
  Fixing leaked_handle in create-image.c
  Fixing leaked_handle in l2cap-tester.c
  Fixing resource leak in mesh/mesh-db.c
  Fixing leaked_handle in obex-client-tool.c
  Fixing use after free in src/device.c
  Fixing memory leak in pbap.c
  Fixing possible use_after_free in meshctl.c
  Fixing use_after_free in prov-db.c

 client/gatt.c             | 11 ++++++++---
 mesh/appkey.c             |  8 ++++++--
 monitor/jlink.c           |  5 ++++-
 obexd/client/pbap.c       |  5 +++--
 plugins/sixaxis.c         |  9 +++++++--
 src/device.c              |  1 +
 tools/cltest.c            |  1 +
 tools/create-image.c      |  7 +++----
 tools/l2cap-tester.c      |  1 +
 tools/mesh-gatt/prov-db.c |  3 ++-
 tools/mesh/mesh-db.c      |  2 ++
 tools/meshctl.c           |  1 -
 tools/obex-client-tool.c  |  1 +
 13 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.26.2


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

* [Bluez V2 01/13] Fixing memory leak issue in gatt.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 02/13] Fixing memory leakage in appkey.c Gopal Tiwari
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing the static tool analysis using coverity tool 
found following reports

Error: RESOURCE_LEAK (CWE-772):
bluez-5.64/client/gatt.c:1531: leaked_storage: Variable "service" 
going out of scope leaks the storage it points to.

Error: RESOURCE_LEAK (CWE-772):
bluez-5.64/client/gatt.c:2626: leaked_storage: Variable "chrc" 
going out of scope leaks the storage it points to.

Error: RESOURCE_LEAK (CWE-772):
bluez-5.64/client/gatt.c:2906: leaked_storage: Variable "desc" 
going out of scope leaks the storage it points to.

Fixing them.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 client/gatt.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index 13872c794..4c1efaf75 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -1527,8 +1527,10 @@ void gatt_register_service(DBusConnection *conn, GDBusProxy *proxy,
 
 	if (argc > 2) {
 		service->handle = parse_handle(argv[2]);
-		if (!service->handle)
+		if (!service->handle) {
+			service_free(service);
 			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
 	}
 
 	if (g_dbus_register_interface(conn, service->path,
@@ -2622,8 +2624,10 @@ void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
 
 	if (argc > 3) {
 		chrc->handle = parse_handle(argv[3]);
-		if (!chrc->handle)
+		if (!chrc->handle) {
+			chrc_free(chrc);
 			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
 	}
 
 	if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
@@ -2902,8 +2906,10 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
 
 	if (argc > 3) {
 		desc->handle = parse_handle(argv[3]);
-		if (!desc->handle)
+		if (!desc->handle) {
+			desc_free(desc);
 			return bt_shell_noninteractive_quit(EXIT_FAILURE);
+		}
 	}
 
 	if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
-- 
2.26.2


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

* [Bluez V2 02/13] Fixing memory leakage in appkey.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 01/13] Fixing memory leak issue in gatt.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 03/13] Fixing memory leak in jlink.c Gopal Tiwari
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing the static analysis using the coverity tool found
following memory leak reports

bluez-5.64/mesh/appkey.c:143: leaked_storage: Variable "key" going 
out of scope leaks the storage it points to.

Error: RESOURCE_LEAK (CWE-772):
bluez-5.64/mesh/appkey.c:146: leaked_storage: Variable "key" going 
out of scope leaks the storage it points to.

Fixing them.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 mesh/appkey.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mesh/appkey.c b/mesh/appkey.c
index 5088a1812..52fed8c31 100644
--- a/mesh/appkey.c
+++ b/mesh/appkey.c
@@ -139,11 +139,15 @@ bool appkey_key_init(struct mesh_net *net, uint16_t net_idx, uint16_t app_idx,
 	key->net_idx = net_idx;
 	key->app_idx = app_idx;
 
-	if (key_value && !set_key(key, app_idx, key_value, false))
+	if (key_value && !set_key(key, app_idx, key_value, false)) {
+		appkey_key_free(key);
 		return false;
+	}
 
-	if (new_key_value && !set_key(key, app_idx, new_key_value, true))
+	if (new_key_value && !set_key(key, app_idx, new_key_value, true)) {
+		appkey_key_free(key);
 		return false;
+	}
 
 	l_queue_push_tail(app_keys, key);
 
-- 
2.26.2


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

* [Bluez V2 03/13] Fixing memory leak in jlink.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 01/13] Fixing memory leak issue in gatt.c Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 02/13] Fixing memory leakage in appkey.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 04/13] Fixing memory leak in sixaxis.c Gopal Tiwari
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity
found following reports for resouse leak

bluez-5.64/monitor/jlink.c:111: leaked_storage: Variable "so"
going out of scope leaks the storage it points to.

bluez-5.64/monitor/jlink.c:113: leaked_storage: Variable "so"
going out of scope leaks the storage it points to.

Fixing them.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 monitor/jlink.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/monitor/jlink.c b/monitor/jlink.c
index 9aaa4ebd8..f1d8ce660 100644
--- a/monitor/jlink.c
+++ b/monitor/jlink.c
@@ -107,9 +107,12 @@ int jlink_init(void)
 			!jlink.tif_select || !jlink.setspeed ||
 			!jlink.connect || !jlink.getsn ||
 			!jlink.emu_getproductname ||
-			!jlink.rtterminal_control || !jlink.rtterminal_read)
+			!jlink.rtterminal_control || !jlink.rtterminal_read) {
+		dlclose(so);
 		return -EIO;
+	}
 
+	dlclose(so);
 	return 0;
 }
 
-- 
2.26.2


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

* [Bluez V2 04/13] Fixing memory leak in sixaxis.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (2 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 03/13] Fixing memory leak in jlink.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 05/13] Fixing leaked_handle in cltest.c Gopal Tiwari
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity
found following reports for resouse leak

bluez-5.64/plugins/sixaxis.c:425: alloc_arg:
"get_pairing_type_for_device" allocates memory that is 
stored into "sysfs_path".

bluez-5.64/plugins/sixaxis.c:428: leaked_storage: Variable "sysfs_path"
going out of scope leaks the storage it points to.

Fixing them.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 plugins/sixaxis.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/plugins/sixaxis.c b/plugins/sixaxis.c
index ddecbcccb..10cf15948 100644
--- a/plugins/sixaxis.c
+++ b/plugins/sixaxis.c
@@ -424,10 +424,15 @@ static void device_added(struct udev_device *udevice)
 
 	cp = get_pairing_type_for_device(udevice, &bus, &sysfs_path);
 	if (!cp || (cp->type != CABLE_PAIRING_SIXAXIS &&
-				cp->type != CABLE_PAIRING_DS4))
+				cp->type != CABLE_PAIRING_DS4)) {
+		g_free(sysfs_path);
 		return;
-	if (bus != BUS_USB)
+	}
+
+	if (bus != BUS_USB) {
+		g_free(sysfs_path);
 		return;
+	}
 
 	info("sixaxis: compatible device connected: %s (%04X:%04X %s)",
 				cp->name, cp->vid, cp->pid, sysfs_path);
-- 
2.26.2


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

* [Bluez V2 05/13] Fixing leaked_handle in cltest.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (3 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 04/13] Fixing memory leak in sixaxis.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 06/13] Fixing leaked_handle in create-image.c Gopal Tiwari
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity found 
following reports for resouse leak

bluez-5.64/tools/cltest.c:75: leaked_handle: Handle variable "fd" 
going out of scope leaks the handle.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/cltest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/cltest.c b/tools/cltest.c
index 2766fcd23..250c93cc7 100644
--- a/tools/cltest.c
+++ b/tools/cltest.c
@@ -72,6 +72,7 @@ static bool send_message(const bdaddr_t *src, const bdaddr_t *dst,
 		return false;
 	}
 
+	close(fd);
 	return true;
 }
 
-- 
2.26.2


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

* [Bluez V2 06/13] Fixing leaked_handle in create-image.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (4 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 05/13] Fixing leaked_handle in cltest.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 07/13] Fixing leaked_handle in l2cap-tester.c Gopal Tiwari
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity found following 
reports for resouse leak

bluez-5.64/tools/create-image.c:124: leaked_storage: Variable "map" 
going out of scope leaks the storage it points to.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/create-image.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/create-image.c b/tools/create-image.c
index aba940da7..90cd87315 100644
--- a/tools/create-image.c
+++ b/tools/create-image.c
@@ -97,12 +97,13 @@ static void write_block(FILE *fp, const char *pathname, unsigned int ino,
 
 	map = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
 	if (!map || map == MAP_FAILED) {
-		close(fd);
-		fd = -1;
 		map = NULL;
 		st.st_size = 0;
         }
 
+	close(fd);
+	fd = -1;
+
 done:
 	fprintf(fp, HDR_FMT, HDR_MAGIC, ino, mode, 0, 0, 1, 0,
 		(uintmax_t) st.st_size, 0, 0, 0, 0, namelen + 1, 0, name);
@@ -117,9 +118,7 @@ done:
 		pad = 3 - ((st.st_size + 3) % 4);
 		for (i = 0; i < pad; i++)
 			fputc(0, fp);
-
 		munmap(map, st.st_size);
-		close(fd);
 	}
 }
 
-- 
2.26.2


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

* [Bluez V2 07/13] Fixing leaked_handle in l2cap-tester.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (5 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 06/13] Fixing leaked_handle in create-image.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 08/13] Fixing resource leak in mesh/mesh-db.c Gopal Tiwari
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity found following 
reports for resouse leak

bluez-5.64/tools/l2cap-tester.c:1712: leaked_handle: Handle variable 
"new_sk" going out of scope leaks the handle.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/l2cap-tester.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
index d78b1e29c..3f0464013 100644
--- a/tools/l2cap-tester.c
+++ b/tools/l2cap-tester.c
@@ -1709,6 +1709,7 @@ static gboolean l2cap_listen_cb(GIOChannel *io, GIOCondition cond,
 
 	if (!check_mtu(data, new_sk)) {
 		tester_test_failed();
+		close(new_sk);
 		return FALSE;
 	}
 
-- 
2.26.2


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

* [Bluez V2 08/13] Fixing resource leak in mesh/mesh-db.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (6 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 07/13] Fixing leaked_handle in l2cap-tester.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 09/13] Fixing leaked_handle in obex-client-tool.c Gopal Tiwari
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity found following 
reports for resouse leak

bluez-5.64/tools/mesh/mesh-db.c:2388: leaked_handle: Handle variable 
"fd" going out of scope leaks the handle.

bluez-5.64/tools/mesh/mesh-db.c:2388: leaked_storage: Variable "str" 
going out of scope leaks the storage it points to.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/mesh/mesh-db.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/mesh/mesh-db.c b/tools/mesh/mesh-db.c
index fa11837df..896ff722c 100644
--- a/tools/mesh/mesh-db.c
+++ b/tools/mesh/mesh-db.c
@@ -2384,6 +2384,8 @@ bool mesh_db_load(const char *fname)
 
 	sz = read(fd, str, st.st_size);
 	if (sz != st.st_size) {
+		close(fd);
+		l_free(str);
 		l_error("Failed to read configuration file %s", fname);
 		return false;
 	}
-- 
2.26.2


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

* [Bluez V2 09/13] Fixing leaked_handle in obex-client-tool.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (7 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 08/13] Fixing resource leak in mesh/mesh-db.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 10/13] Fixing use after free in src/device.c Gopal Tiwari
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

While performing static tool analysis using coverity found following 
reports for resouse leak

bluez-5.64/tools/obex-client-tool.c:315: leaked_handle: Handle variable 
"sk" going out of scope leaks the handle.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/obex-client-tool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/obex-client-tool.c b/tools/obex-client-tool.c
index ab9332896..cb0e41247 100644
--- a/tools/obex-client-tool.c
+++ b/tools/obex-client-tool.c
@@ -312,6 +312,7 @@ static GIOChannel *unix_connect(GObexTransportType transport)
 	if (connect(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
 		err = errno;
 		g_printerr("connect: %s (%d)\n", strerror(err), err);
+		close(sk);
 		return NULL;
 	}
 
-- 
2.26.2


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

* [Bluez V2 10/13] Fixing use after free in src/device.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (8 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 09/13] Fixing leaked_handle in obex-client-tool.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31 20:22   ` Luiz Augusto von Dentz
  2022-05-31  7:41 ` [Bluez V2 11/13] Fixing memory leak in pbap.c Gopal Tiwari
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Following traces reported by covirty tool

Error: USE_AFTER_FREE (CWE-416):
bluez-5.64/src/device.c:2962: path: Condition 
"!dbus_message_get_args(msg, NULL, 0 /* (int)0 */)", taking false branch.

bluez-5.64/src/device.c:2965: path: 
Condition "device->bonding", taking false branch.

bluez-5.64/src/device.c:2968: path: 
Condition "device->bredr_state.bonded", taking true branch.

bluez-5.64/src/device.c:2969: path: Falling through to end of 
if statement.

bluez-5.64/src/device.c:2977: path: Condition "state->bonded", 
taking false branch.

bluez-5.64/src/device.c:2983: path: Condition "agent", taking 
true branch.

bluez-5.64/src/device.c:2984: path: Falling through to end of 
if statement.

bluez-5.64/src/device.c:2990: path: Condition "agent", taking 
true branch.

bluez-5.64/src/device.c:3005: path: Condition "bdaddr_type != 0", 
taking true branch.

bluez-5.64/src/device.c:3006: path: 

Condition "!state->connected", taking true branch.

bluez-5.64/src/device.c:3006: path: 
Condition "btd_le_connect_before_pairing()", taking true branch.
bluez-5.64/src/device.c:3007: freed_arg: "device_connect_le" frees 
"device->bonding".

bluez-5.64/src/device.c:3007: path: Falling through to end of 
if statement.

bluez-5.64/src/device.c:3012: path: Falling through to end of 
if statement.

bluez-5.64/src/device.c:3017: path: Condition "err < 0", 
taking true branch.

bluez-5.64/src/device.c:3018: double_free: Calling "bonding_request_free" 
frees pointer "device->bonding" which has already been freed.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/device.c b/src/device.c
index 8dc12d026..a0e5d40db 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2942,6 +2942,7 @@ static void bonding_request_free(struct bonding_req *bonding)
 		bonding->device->bonding = NULL;
 
 	g_free(bonding);
+	bonding = NULL;
 }
 
 static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
-- 
2.26.2


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

* [Bluez V2 11/13] Fixing memory leak in pbap.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (9 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 10/13] Fixing use after free in src/device.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 12/13] Fixing possible use_after_free in meshctl.c Gopal Tiwari
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Reported by coverity tool as follows:

bluez-5.64/obexd/client/pbap.c:929: leaked_storage: Variable "apparam" 
going out of scope leaks the storage it points to.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 obexd/client/pbap.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/obexd/client/pbap.c b/obexd/client/pbap.c
index 1a2bacc9f..1ed8c68ec 100644
--- a/obexd/client/pbap.c
+++ b/obexd/client/pbap.c
@@ -925,10 +925,11 @@ static DBusMessage *pbap_search(DBusConnection *connection,
 		return g_dbus_create_error(message,
 				ERROR_INTERFACE ".InvalidArguments", NULL);
 
-	if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_STRING)
+	if (dbus_message_iter_get_arg_type(&args) != DBUS_TYPE_STRING) {
+		g_obex_apparam_free(apparam);
 		return g_dbus_create_error(message,
 				ERROR_INTERFACE ".InvalidArguments", NULL);
-
+	}
 	dbus_message_iter_get_basic(&args, &value);
 	dbus_message_iter_next(&args);
 
-- 
2.26.2


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

* [Bluez V2 12/13] Fixing possible use_after_free in meshctl.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (10 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 11/13] Fixing memory leak in pbap.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31  7:41 ` [Bluez V2 13/13] Fixing use_after_free in prov-db.c Gopal Tiwari
  2022-05-31 20:30 ` [Bluez 00/13] Fixing memory leak, leaked_handle and use_after patchwork-bot+bluetooth
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Reported by coverity tool as follows :

bluez-5.64/tools/meshctl.c:1968: freed_arg: "g_free" frees "mesh_dir".

bluez-5.64/tools/meshctl.c:2018: double_free: Calling "g_free" frees 
pointer "mesh_dir" which has already been freed.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/meshctl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/meshctl.c b/tools/meshctl.c
index 18e20c40d..38ffd35f3 100644
--- a/tools/meshctl.c
+++ b/tools/meshctl.c
@@ -2015,7 +2015,6 @@ int main(int argc, char *argv[])
 
 fail:
 	bt_shell_cleanup();
-	g_free(mesh_dir);
 
 	return EXIT_FAILURE;
 }
-- 
2.26.2


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

* [Bluez V2 13/13] Fixing use_after_free in prov-db.c
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (11 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 12/13] Fixing possible use_after_free in meshctl.c Gopal Tiwari
@ 2022-05-31  7:41 ` Gopal Tiwari
  2022-05-31 20:30 ` [Bluez 00/13] Fixing memory leak, leaked_handle and use_after patchwork-bot+bluetooth
  13 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-31  7:41 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Following scenario happens when prov is false and we have double free as
mentioned in the below

bluez-5.64/tools/mesh-gatt/prov-db.c:847: freed_arg: "g_free" frees 
"in_str".

bluez-5.64/tools/mesh-gatt/prov-db.c:867: double_free: Calling "g_free"
frees pointer "in_str" which has already been freed.

Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
---
 tools/mesh-gatt/prov-db.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/mesh-gatt/prov-db.c b/tools/mesh-gatt/prov-db.c
index 2fb08f799..a5b6997e0 100644
--- a/tools/mesh-gatt/prov-db.c
+++ b/tools/mesh-gatt/prov-db.c
@@ -859,7 +859,8 @@ bool prov_db_local_set_iv_index(uint32_t iv_index, bool update, bool prov)
 
 		set_local_iv_index(jmain, iv_index, update);
 		prov_file_write(jmain, false);
-	}
+	} else
+		return true;
 
 	res = true;
 done:
-- 
2.26.2


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

* Re: [Bluez V2 10/13] Fixing use after free in src/device.c
  2022-05-31  7:41 ` [Bluez V2 10/13] Fixing use after free in src/device.c Gopal Tiwari
@ 2022-05-31 20:22   ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 17+ messages in thread
From: Luiz Augusto von Dentz @ 2022-05-31 20:22 UTC (permalink / raw)
  To: Gopal Tiwari; +Cc: linux-bluetooth, Gopal Tiwari

Hi Gopal,

On Tue, May 31, 2022 at 12:42 AM Gopal Tiwari
<gopalkrishna.tiwari@gmail.com> wrote:
>
> From: Gopal Tiwari <gtiwari@redhat.com>
>
> Following traces reported by covirty tool
>
> Error: USE_AFTER_FREE (CWE-416):
> bluez-5.64/src/device.c:2962: path: Condition
> "!dbus_message_get_args(msg, NULL, 0 /* (int)0 */)", taking false branch.
>
> bluez-5.64/src/device.c:2965: path:
> Condition "device->bonding", taking false branch.
>
> bluez-5.64/src/device.c:2968: path:
> Condition "device->bredr_state.bonded", taking true branch.
>
> bluez-5.64/src/device.c:2969: path: Falling through to end of
> if statement.
>
> bluez-5.64/src/device.c:2977: path: Condition "state->bonded",
> taking false branch.
>
> bluez-5.64/src/device.c:2983: path: Condition "agent", taking
> true branch.
>
> bluez-5.64/src/device.c:2984: path: Falling through to end of
> if statement.
>
> bluez-5.64/src/device.c:2990: path: Condition "agent", taking
> true branch.
>
> bluez-5.64/src/device.c:3005: path: Condition "bdaddr_type != 0",
> taking true branch.
>
> bluez-5.64/src/device.c:3006: path:
>
> Condition "!state->connected", taking true branch.
>
> bluez-5.64/src/device.c:3006: path:
> Condition "btd_le_connect_before_pairing()", taking true branch.
> bluez-5.64/src/device.c:3007: freed_arg: "device_connect_le" frees
> "device->bonding".
>
> bluez-5.64/src/device.c:3007: path: Falling through to end of
> if statement.
>
> bluez-5.64/src/device.c:3012: path: Falling through to end of
> if statement.
>
> bluez-5.64/src/device.c:3017: path: Condition "err < 0",
> taking true branch.
>
> bluez-5.64/src/device.c:3018: double_free: Calling "bonding_request_free"
> frees pointer "device->bonding" which has already been freed.
>
> Signed-off-by: Gopal Tiwari <gtiwari@redhat.com>
> ---
>  src/device.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/src/device.c b/src/device.c
> index 8dc12d026..a0e5d40db 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -2942,6 +2942,7 @@ static void bonding_request_free(struct bonding_req *bonding)
>                 bonding->device->bonding = NULL;
>
>         g_free(bonding);
> +       bonding = NULL;

I don't think this fixes anything really, since bonding variable goes
out of scope this won't change anything, in fact this seems to be a
false positive since device->bonding shall be set to NULL in the if
statement just above any other call to bonding_request_free will bail
out when checking !bonding, it would be a problem if and only if the
code was using bonding pointer directly instead of device->bonding
with bonding_request_free.

>  }
>
>  static DBusMessage *pair_device(DBusConnection *conn, DBusMessage *msg,
> --
> 2.26.2
>


-- 
Luiz Augusto von Dentz

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

* Re: [Bluez 00/13] Fixing memory leak, leaked_handle and use_after
  2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
                   ` (12 preceding siblings ...)
  2022-05-31  7:41 ` [Bluez V2 13/13] Fixing use_after_free in prov-db.c Gopal Tiwari
@ 2022-05-31 20:30 ` patchwork-bot+bluetooth
  13 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+bluetooth @ 2022-05-31 20:30 UTC (permalink / raw)
  To: Gopal Tiwari; +Cc: linux-bluetooth, luiz.dentz, gtiwari

Hello:

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

On Tue, 31 May 2022 13:11:04 +0530 you wrote:
> From: Gopal Tiwari <gtiwari@redhat.com>
> 
> Following fixes are reported by coverity tool.
> 
> Gopal Tiwari (13):
>   Fixing memory leak issue in gatt.c
>   Fixing memory leakage in appkey.c
>   Fixing memory leak in jlink.c
>   Fixing memory leak in sixaxis.c
>   Fixing leaked_handle in cltest.c
>   Fixing leaked_handle in create-image.c
>   Fixing leaked_handle in l2cap-tester.c
>   Fixing resource leak in mesh/mesh-db.c
>   Fixing leaked_handle in obex-client-tool.c
>   Fixing use after free in src/device.c
>   Fixing memory leak in pbap.c
>   Fixing possible use_after_free in meshctl.c
>   Fixing use_after_free in prov-db.c
> 
> [...]

Here is the summary with links:
  - [Bluez,V2,01/13] Fixing memory leak issue in gatt.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=b4233bca1815
  - [Bluez,V2,02/13] Fixing memory leakage in appkey.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5eb96b3ec854
  - [Bluez,V2,03/13] Fixing memory leak in jlink.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=6f02010ce004
  - [Bluez,V2,04/13] Fixing memory leak in sixaxis.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=fc57aa92a4f3
  - [Bluez,V2,05/13] Fixing leaked_handle in cltest.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f4743109f381
  - [Bluez,V2,06/13] Fixing leaked_handle in create-image.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=4ae130455b17
  - [Bluez,V2,07/13] Fixing leaked_handle in l2cap-tester.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=4334be027ae1
  - [Bluez,V2,08/13] Fixing resource leak in mesh/mesh-db.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=35cbfd966094
  - [Bluez,V2,09/13] Fixing leaked_handle in obex-client-tool.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=39b638526d9a
  - [Bluez,V2,10/13] Fixing use after free in src/device.c
    (no matching commit)
  - [Bluez,V2,11/13] Fixing memory leak in pbap.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=06d3c7429ad6
  - [Bluez,V2,12/13] Fixing possible use_after_free in meshctl.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=56bda20ce9e3
  - [Bluez,V2,13/13] Fixing use_after_free in prov-db.c
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5cdaeaefc350

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] 17+ messages in thread

* [Bluez 00/13] Fixing memory leak, leaked_handle and use_after
@ 2022-05-30  8:11 Gopal Tiwari
  0 siblings, 0 replies; 17+ messages in thread
From: Gopal Tiwari @ 2022-05-30  8:11 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: luiz.dentz, gtiwari

From: Gopal Tiwari <gtiwari@redhat.com>

Following fixes are reported by coverity tool.  

Gopal Tiwari (13):
  Fixing memory leak issue in gatt.c
  Fixing memory leakage in appkey.c
  Fixing memroy leak in jlink.c
  Fixing memory leak in sixaxis.c
  Fixing leaked_handle in cltest.c
  Fixing leaked_handle in create-image.c
  Fixing leaked_handle in l2cap-tester.c
  Fixing resource leak in mesh/mesh-db.c
  Fixing leaked_handle in obex-client-tool.c
  Fixing use after free in src/device.c
  Fixing memory leak in pbap.c
  Fixing possible use_after_free in meshctl.c
  Fixing use_after_free in prov-db.c

 client/gatt.c             | 11 ++++++++---
 mesh/appkey.c             |  8 ++++++--
 monitor/jlink.c           |  5 ++++-
 obexd/client/pbap.c       |  5 +++--
 plugins/sixaxis.c         |  9 +++++++--
 src/device.c              |  1 +
 tools/cltest.c            |  1 +
 tools/create-image.c      |  7 +++----
 tools/l2cap-tester.c      |  1 +
 tools/mesh-gatt/prov-db.c |  3 ++-
 tools/mesh/mesh-db.c      |  2 ++
 tools/meshctl.c           |  1 -
 tools/obex-client-tool.c  |  1 +
 13 files changed, 39 insertions(+), 16 deletions(-)

-- 
2.26.2


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

end of thread, other threads:[~2022-05-31 20:30 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-31  7:41 [Bluez 00/13] Fixing memory leak, leaked_handle and use_after Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 01/13] Fixing memory leak issue in gatt.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 02/13] Fixing memory leakage in appkey.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 03/13] Fixing memory leak in jlink.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 04/13] Fixing memory leak in sixaxis.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 05/13] Fixing leaked_handle in cltest.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 06/13] Fixing leaked_handle in create-image.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 07/13] Fixing leaked_handle in l2cap-tester.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 08/13] Fixing resource leak in mesh/mesh-db.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 09/13] Fixing leaked_handle in obex-client-tool.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 10/13] Fixing use after free in src/device.c Gopal Tiwari
2022-05-31 20:22   ` Luiz Augusto von Dentz
2022-05-31  7:41 ` [Bluez V2 11/13] Fixing memory leak in pbap.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 12/13] Fixing possible use_after_free in meshctl.c Gopal Tiwari
2022-05-31  7:41 ` [Bluez V2 13/13] Fixing use_after_free in prov-db.c Gopal Tiwari
2022-05-31 20:30 ` [Bluez 00/13] Fixing memory leak, leaked_handle and use_after patchwork-bot+bluetooth
  -- strict thread matches above, loose matches on Subject: below --
2022-05-30  8:11 Gopal Tiwari

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.