Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. [PATCH v2 0/2] ofono: Fix segfault during modem_removed
      (Lukasz Nowak)
   2. [PATCH v2 1/2] ofono: Fix segfault during modem_removed
      (Lukasz Nowak)
   3. [PATCH v2 2/2] ofono: Clear dbus pending call pointers
      (Lukasz Nowak)


----------------------------------------------------------------------

Message: 1
Date: Tue,  4 Apr 2017 20:36:58 +0100
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH v2 0/2] ofono: Fix segfault during modem_removed
Message-ID: <[email protected]>

From: Lukasz Nowak <[email protected]>

Changes since v1:
- split patch into two separate commits
- added more information to commit messages

The second patch is technically not required, after the first patch.
But it might save somebody 2 days of debugging memory corruption in the future,
in case a different path emerges, which can use the non-cleared pointers to
already freed memory.

Lukasz Nowak (2):
  ofono: Fix segfault during modem_removed
  ofono: Clear dbus pending call pointers

 plugins/ofono.c | 9 +++++++++
 1 file changed, 9 insertions(+)

-- 
2.7.4



------------------------------

Message: 2
Date: Tue,  4 Apr 2017 20:36:59 +0100
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH v2 1/2] ofono: Fix segfault during modem_removed
Message-ID: <[email protected]>

From: Lukasz Nowak <[email protected]>

Network notifcations (network_connect, network_disconnect) can occur
after an asynchronous modem_removed dbus signal. This can happen in the
middle of resource de-allocation procedure in remove_modem (for instance
as a result of remove_all_contexts call).

In this scenario, the network operations should not send any further
dbus messages, as the modem is already removed on the other side.

This state is indicated by the modem_hash object - modem_removed
starts by taking out the removed modem instance from the hash.

The network notifications during modem_removed were causing
segmentation faults, by using already freed modem objects.

Problematic sequence was:
- modem_removed
- remove_modem
- remove_all_contexts
- remove_cm_context
- remove_network
- connman_device_remove_network
- network_disconnect
- context_set_active
- set_property
---
 plugins/ofono.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index eb834f5..237d257 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -2697,6 +2697,9 @@ static int network_connect(struct connman_network 
*network)
 
        DBG("%s network %p", modem->path, network);
 
+       if (!g_hash_table_lookup(modem_hash, modem->path))
+               return -ENODEV;
+
        context = get_context_with_network(modem->context_list, network);
        if (!context)
                return -ENODEV;
@@ -2718,6 +2721,9 @@ static int network_disconnect(struct connman_network 
*network)
 
        DBG("%s network %p", modem->path, network);
 
+       if (!g_hash_table_lookup(modem_hash, modem->path))
+               return -ENODEV;
+
        context = get_context_with_network(modem->context_list, network);
        if (!context)
                return -ENODEV;
-- 
2.7.4



------------------------------

Message: 3
Date: Tue,  4 Apr 2017 20:37:00 +0100
From: Lukasz Nowak <[email protected]>
To: [email protected]
Subject: [PATCH v2 2/2] ofono: Clear dbus pending call pointers
Message-ID: <[email protected]>

From: Lukasz Nowak <[email protected]>

This is an additional protection against double free operation on
the DbusPendingCall objects' memory.

remove_modem() de-allocates memory by calling dbus_pending_call_unref().
But the subsequent calls (e.g. remove_all_contexts) may result in other
modules calling (e.g. network) calling the ofono plugin back and
potentially calling dbus_pending_call_unref() on the same pointer,
causing modification of already freed memory (and a second free).

With musl this results in corruption of internal malloc structures and
a segmentation fault.

While all the ofono plugin functions should be prevented to operate
after remove_modem was called, as a protective measure clear the
DbusPendingCall pointers. Debugging memory corruption caused by re-use
of those pointers can be very time consuming.
---
 plugins/ofono.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/plugins/ofono.c b/plugins/ofono.c
index 237d257..78f8f19 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -2498,16 +2498,19 @@ static void remove_modem(gpointer data)
        if (modem->call_set_property) {
                dbus_pending_call_cancel(modem->call_set_property);
                dbus_pending_call_unref(modem->call_set_property);
+               modem->call_set_property = NULL;
        }
 
        if (modem->call_get_properties) {
                dbus_pending_call_cancel(modem->call_get_properties);
                dbus_pending_call_unref(modem->call_get_properties);
+               modem->call_get_properties = NULL;
        }
 
        if (modem->call_get_contexts) {
                dbus_pending_call_cancel(modem->call_get_contexts);
                dbus_pending_call_unref(modem->call_get_contexts);
+               modem->call_get_contexts = NULL;
        }
 
        /* Must remove the contexts before the device */
-- 
2.7.4



------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 18, Issue 4
**************************************

Reply via email to