[PATCH v2] network: Fix use-after-free caused by Scan() in poor reception.

[PATCH v2] network: Fix use-after-free caused by Scan() in poor reception.

[John Ernberg]

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

From: John Ernberg <john.ernberg@actia.se>

The area where I could test this behavior has gotten better reception now so I
cannot test this patch as thoroughly as I'd like but it seems to work.
Please be extra strict during the review to minimize the risk of something
being by this.

John Ernberg (1):
  network: Fix use-after-free caused by Scan() in poor reception.

 src/network.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH v2] network: Fix use-after-free caused by Scan() in poor reception.

[John Ernberg]

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

From: John Ernberg <john.ernberg@actia.se>

When issuing a Scan() in poor reception while attached to an operator it's
fully possible to get no results, which causes the attached operator to be
cleaned up. In certain scenarios this would cause a use-after-free as there
are still references to this operator.
Transfer the attached operator to the new list regardless of removal caused
by the Scan() results.
---
 src/network.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/network.c b/src/network.c
index 1dddcac..8ad11d3 100644
--- a/src/network.c
+++ b/src/network.c
@@ -710,6 +710,7 @@ static gboolean update_operator_list(struct ofono_netreg *netreg, int total,
 	GSList *o;
 	GSList *compressed;
 	GSList *c;
+	struct network_operator_data *current_op = NULL;
 	gboolean changed = FALSE;
 
 	compressed = compress_operator_list(list, total);
@@ -754,8 +755,19 @@ static gboolean update_operator_list(struct ofono_netreg *netreg, int total,
 	if (netreg->operator_list)
 		changed = TRUE;
 
-	for (o = netreg->operator_list; o; o = o->next)
-		network_operator_dbus_unregister(netreg, o->data);
+	for (o = netreg->operator_list; o; o = o->next) {
+		struct network_operator_data *op = o->data;
+		if (op != op->netreg->current_operator)
+			network_operator_dbus_unregister(netreg, op);
+		else
+			current_op = op;
+	}
+
+	if (current_op) {
+		n = g_slist_prepend(n, current_op);
+		netreg->operator_list =
+			g_slist_remove(netreg->operator_list, current_op);
+	}
 
 	g_slist_free(netreg->operator_list);
 
-- 
1.9.1

Re: [PATCH v2] network: Fix use-after-free caused by Scan() in poor reception.

[Denis Kenzior]

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

Hi John,

On 01/07/2016 04:46 AM, John Ernberg wrote:
> From: John Ernberg <john.ernberg@actia.se>
>
> When issuing a Scan() in poor reception while attached to an operator it's
> fully possible to get no results, which causes the attached operator to be
> cleaned up. In certain scenarios this would cause a use-after-free as there
> are still references to this operator.
> Transfer the attached operator to the new list regardless of removal caused
> by the Scan() results.
> ---
>   src/network.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>

Looks good to me.  I tweaked the commit header slightly and applied the 
patch.

Thanks!

Regards,
-Denis