[PATCH 1/3] voicecall: Fix memory leak

Open Source Telephony
 help / color / mirror / Atom feed

* [PATCH 1/3] voicecall: Fix memory leak
@ 2024-02-27 15:33 Denis Kenzior
  2024-02-27 15:33 ` [PATCH 2/3] voicecall: Fix use after free Denis Kenzior
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Denis Kenzior @ 2024-02-27 15:33 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

'number' is obtained using g_key_file_get_string (which returns a newly
allocated string), but is never freed.
---
 src/voicecall.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index d10fe15a423d..f979c46fe132 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -3462,7 +3462,7 @@ static void emulator_dial_callback(const struct ofono_error *error, void *data)
 	struct ofono_voicecall *vc = data;
 	gboolean need_to_emit;
 	struct voicecall *v;
-	const char *number;
+	char *number;
 	GError *err = NULL;
 
 	number = g_key_file_get_string(vc->settings, SETTINGS_GROUP,
@@ -3484,6 +3484,8 @@ static void emulator_dial_callback(const struct ofono_error *error, void *data)
 
 	if (need_to_emit)
 		voicecalls_emit_call_added(vc, v);
+
+	g_free(number);
 }
 
 static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
@@ -3585,7 +3587,7 @@ static void emulator_bldn_cb(struct ofono_emulator *em,
 			struct ofono_emulator_request *req, void *userdata)
 {
 	struct ofono_voicecall *vc = userdata;
-	const char *number;
+	char *number = NULL;
 	struct ofono_error result;
 	GError *error = NULL;
 
@@ -3608,6 +3610,8 @@ fail:
 		result.type = OFONO_ERROR_TYPE_FAILURE;
 		ofono_emulator_send_final(em, &result);
 	};
+
+	g_free(number);
 }
 
 static void emulator_hfp_watch(struct ofono_atom *atom,
-- 
2.43.0


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

* [PATCH 2/3] voicecall: Fix use after free
  2024-02-27 15:33 [PATCH 1/3] voicecall: Fix memory leak Denis Kenzior
@ 2024-02-27 15:33 ` Denis Kenzior
  2024-02-27 15:33 ` [PATCH 3/3] lte: Fix invalid cleanup Denis Kenzior
  2024-02-27 17:50 ` [PATCH 1/3] voicecall: Fix memory leak patchwork-bot+ofono
  2 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2024-02-27 15:33 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

If voicecall_dbus_register is not successful, it will invoke
voicecall_destroy, which frees the voicecall and associated data.  Make
sure to return early if this happens.

While here, convert g_memdup2 and g_try_new0 use to ell.
---
 src/voicecall.c | 70 +++++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 49 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index f979c46fe132..4ce161b54600 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -671,11 +671,7 @@ static const GDBusSignalTable voicecall_signals[] = {
 static struct voicecall *voicecall_create(struct ofono_voicecall *vc,
 						struct ofono_call *call)
 {
-	struct voicecall *v;
-
-	v = g_try_new0(struct voicecall, 1);
-	if (v == NULL)
-		return NULL;
+	struct voicecall *v = l_new(struct voicecall, 1);
 
 	v->call = call;
 	v->vc = vc;
@@ -687,10 +683,10 @@ static void voicecall_destroy(gpointer userdata)
 {
 	struct voicecall *voicecall = (struct voicecall *)userdata;
 
-	g_free(voicecall->call);
+	l_free(voicecall->call);
 	l_free(voicecall->message);
 
-	g_free(voicecall);
+	l_free(voicecall);
 }
 
 static const char *voicecall_build_path(struct ofono_voicecall *vc,
@@ -1051,13 +1047,13 @@ static void voicecall_set_call_name(struct voicecall *v,
 						DBUS_TYPE_STRING, &name_str);
 }
 
-static gboolean voicecall_dbus_register(struct voicecall *v)
+static bool voicecall_dbus_register(struct voicecall *v)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path;
 
 	if (v == NULL)
-		return FALSE;
+		return false;
 
 	path = voicecall_build_path(v->vc, v->call);
 
@@ -1068,10 +1064,10 @@ static gboolean voicecall_dbus_register(struct voicecall *v)
 		ofono_error("Could not register VoiceCall %s", path);
 		voicecall_destroy(v);
 
-		return FALSE;
+		return false;
 	}
 
-	return TRUE;
+	return true;
 }
 
 static gboolean voicecall_dbus_unregister(struct ofono_voicecall *vc,
@@ -1357,20 +1353,16 @@ static struct voicecall *synthesize_outgoing_call(struct ofono_voicecall *vc,
 	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 	struct ofono_call *call;
 	struct voicecall *v;
+	unsigned int id;
 
-	call = g_try_new0(struct ofono_call, 1);
-	if (call == NULL)
-		return NULL;
-
-	call->id = __ofono_modem_callid_next(modem);
-
-	if (call->id == 0) {
+	id = __ofono_modem_callid_next(modem);
+	if (!id) {
 		ofono_error("Failed to alloc callid, too many calls");
-		g_free(call);
 		return NULL;
 	}
 
-	__ofono_modem_callid_hold(modem, call->id);
+	call = l_new(struct ofono_call, 1);
+	call->id = id;
 
 	if (number)
 		string_to_phone_number(number, &call->phone_number);
@@ -1380,16 +1372,15 @@ static struct voicecall *synthesize_outgoing_call(struct ofono_voicecall *vc,
 	call->clip_validity = CLIP_VALIDITY_VALID;
 
 	v = voicecall_create(vc, call);
-	if (v == NULL) {
-		g_free(call);
-		return NULL;
-	}
-
 	v->detect_time = time(NULL);
 
 	DBG("Registering new call: %d", call->id);
-	voicecall_dbus_register(v);
+	if (!voicecall_dbus_register(v)) {
+		ofono_error("Unable to register synthetic voice call");
+		return NULL;
+	}
 
+	__ofono_modem_callid_hold(modem, call->id);
 	vc->call_list = g_slist_insert_sorted(vc->call_list, v, call_compare);
 
 	return v;
@@ -2384,7 +2375,7 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 {
 	struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 	GSList *l;
-	struct voicecall *v = NULL;
+	struct voicecall *v;
 	struct ofono_call *newcall;
 
 	DBG("Got a voicecall event, status: %s (%d), id: %u, number: %s"
@@ -2410,19 +2401,8 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 
 	DBG("Did not find a call with id: %d", call->id);
 
-	__ofono_modem_callid_hold(modem, call->id);
-
-	newcall = g_memdup2(call, sizeof(struct ofono_call));
-	if (newcall == NULL) {
-		ofono_error("Unable to allocate call");
-		goto error;
-	}
-
+	newcall = l_memdup(call, sizeof(struct ofono_call));
 	v = voicecall_create(vc, newcall);
-	if (v == NULL) {
-		ofono_error("Unable to allocate voicecall_data");
-		goto error;
-	}
 
 	if (vc->flags & VOICECALL_FLAG_STK_MODEM_CALLSETUP) {
 		struct dial_request *req = vc->dial_req;
@@ -2460,21 +2440,13 @@ void ofono_voicecall_notify(struct ofono_voicecall *vc,
 
 	if (!voicecall_dbus_register(v)) {
 		ofono_error("Unable to register voice call");
-		goto error;
+		return;
 	}
 
+	__ofono_modem_callid_hold(modem, call->id);
 	vc->call_list = g_slist_insert_sorted(vc->call_list, v, call_compare);
 
 	voicecalls_emit_call_added(vc, v);
-
-	return;
-
-error:
-	if (newcall)
-		g_free(newcall);
-
-	if (v)
-		g_free(v);
 }
 
 void ofono_voicecall_mpty_hint(struct ofono_voicecall *vc, unsigned int ids)
-- 
2.43.0


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

* [PATCH 3/3] lte: Fix invalid cleanup
  2024-02-27 15:33 [PATCH 1/3] voicecall: Fix memory leak Denis Kenzior
  2024-02-27 15:33 ` [PATCH 2/3] voicecall: Fix use after free Denis Kenzior
@ 2024-02-27 15:33 ` Denis Kenzior
  2024-02-27 17:50 ` [PATCH 1/3] voicecall: Fix memory leak patchwork-bot+ofono
  2 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2024-02-27 15:33 UTC (permalink / raw)
  To: ofono; +Cc: Denis Kenzior

The intent was to have l_free apply to the settings variable, not the
const ap variable.  Also, while here, fix dereferencing ap if it is
NULL.

Fixes: 69adffb51633 ("lte: Add provisioning support")
---
 src/lte.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/lte.c b/src/lte.c
index 05491c3a1391..d6de408492c7 100644
--- a/src/lte.c
+++ b/src/lte.c
@@ -63,8 +63,8 @@ static bool provision_default_attach_info(struct ofono_lte *lte,
 						const char *mcc, const char *mnc,
 						const char *spn)
 {
-	struct provision_db_entry *settings;
-	_auto_(l_free) const struct provision_db_entry *ap = NULL;
+	_auto_(l_free) struct provision_db_entry *settings = NULL;
+	const struct provision_db_entry *ap = NULL;
 	size_t count;
 	size_t i;
 
@@ -83,7 +83,7 @@ static bool provision_default_attach_info(struct ofono_lte *lte,
 		}
 	}
 
-	if (!is_valid_apn(ap->apn))
+	if (!ap || !is_valid_apn(ap->apn))
 		return false;
 
 	if (ap->username && strlen(ap->username) >
-- 
2.43.0


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

* Re: [PATCH 1/3] voicecall: Fix memory leak
  2024-02-27 15:33 [PATCH 1/3] voicecall: Fix memory leak Denis Kenzior
  2024-02-27 15:33 ` [PATCH 2/3] voicecall: Fix use after free Denis Kenzior
  2024-02-27 15:33 ` [PATCH 3/3] lte: Fix invalid cleanup Denis Kenzior
@ 2024-02-27 17:50 ` patchwork-bot+ofono
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+ofono @ 2024-02-27 17:50 UTC (permalink / raw)
  To: Denis Kenzior; +Cc: ofono

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Tue, 27 Feb 2024 09:33:02 -0600 you wrote:
> 'number' is obtained using g_key_file_get_string (which returns a newly
> allocated string), but is never freed.
> ---
>  src/voicecall.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Here is the summary with links:
  - [1/3] voicecall: Fix memory leak
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=0695b4df2ca4
  - [2/3] voicecall: Fix use after free
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=a46dff5b2476
  - [3/3] lte: Fix invalid cleanup
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=6bf0a2541d43

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

end of thread, other threads:[~2024-02-27 17:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-27 15:33 [PATCH 1/3] voicecall: Fix memory leak Denis Kenzior
2024-02-27 15:33 ` [PATCH 2/3] voicecall: Fix use after free Denis Kenzior
2024-02-27 15:33 ` [PATCH 3/3] lte: Fix invalid cleanup Denis Kenzior
2024-02-27 17:50 ` [PATCH 1/3] voicecall: Fix memory leak patchwork-bot+ofono

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