[PATCH] agent: Check for pointer validity before cancelling agent request
In the case where agent disappears from D-Bus while there are pending requests, request-driver may become null. As it's useless to send a Cancel to a nonexistent agent, not to mention accessing null pointer causes a crash, check for the pointer value before creating a D-Bus message. --- src/agent.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/agent.c b/src/agent.c index a340026..f67b30f 100644 --- a/src/agent.c +++ b/src/agent.c @@ -166,6 +166,9 @@ static int send_cancel_request(struct connman_agent *agent, { DBusMessage *message; + if (!request-driver) + return 0; + DBG(send cancel req to %s %s, agent-owner, agent-path); message = dbus_message_new_method_call(agent-owner, -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] Release references to technologies held by rfkill during cleanup.
Hi, On Tue, 2014-10-21 at 12:27 -0500, David Lechner wrote: --- src/connman.h| 1 + src/rfkill.c | 2 ++ src/technology.c | 15 +++ 3 files changed, 18 insertions(+) diff --git a/src/connman.h b/src/connman.h index da01215..bab013b 100644 --- a/src/connman.h +++ b/src/connman.h @@ -826,6 +826,7 @@ void __connman_service_save(struct connman_service *service); #include connman/notifier.h +void __connman_technology_cleanup_rfkill(void); int __connman_technology_init(void); void __connman_technology_cleanup(void); diff --git a/src/rfkill.c b/src/rfkill.c index 960cfea..2f073a9 100644 --- a/src/rfkill.c +++ b/src/rfkill.c @@ -232,4 +232,6 @@ void __connman_rfkill_cleanup(void) g_io_channel_unref(channel); channel = NULL; + + __connman_technology_cleanup_rfkill(); } diff --git a/src/technology.c b/src/technology.c index d80d9e6..5917c59 100644 --- a/src/technology.c +++ b/src/technology.c @@ -1776,6 +1776,21 @@ int __connman_technology_remove_rfkill(unsigned int index, return 0; } +gboolean find_first(gpointer key, gpointer value, gpointer user_data) +{ + return TRUE; +} + +void __connman_technology_cleanup_rfkill(void) +{ + struct connman_rfkill *rfkill; + + DBG(); + + while (rfkill = g_hash_table_find(rfkill_list, find_first, NULL)) + __connman_technology_remove_rfkill(rfkill-index, rfkill-type); +} + int __connman_technology_init(void) { DBG(); This sort of works, but I'd like to fix the problem in src/technology.c, where the problem manifests itself. So it seems ConnMan is not doing the technology_put() cleanup in the correct place. As you noticed this currently happens only in __connman_technology_remove(), and not on shutdown where only free_rfkill() is called. So the code starting with the line technology = technology_find(type); in __connman_technology_remove() should actually happen in free_rfkill() instead. That way no matter what frees up an rfkill structure also causes a technology_put() where the last one sends off a TechnologRemoved signal before destroying the technology structure. Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/2] test/wait-for-ready: wait for an interface to be ready
Adds a script that exits with success when state changes to ready --- Makefile.am | 2 +- test/wait-for-ready | 41 + 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100755 test/wait-for-ready diff --git a/Makefile.am b/Makefile.am index a574170..af14dcc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -354,7 +354,7 @@ test_scripts = test/get-state test/list-services \ test/test-new-supplicant test/service-move-before \ test/set-global-timeservers test/get-global-timeservers \ test/set-nameservers test/set-domains test/set-timeservers \ - test/set-clock + test/set-clock test/wait-for-ready test_scripts += test/vpn-connect test/vpn-disconnect test/vpn-get \ test/monitor-vpn test/vpn-property diff --git a/test/wait-for-ready b/test/wait-for-ready new file mode 100755 index 000..797317d --- /dev/null +++ b/test/wait-for-ready @@ -0,0 +1,41 @@ +#!/usr/bin/python +import gobject + +import sys +import dbus +import dbus.mainloop.glib +from functools import partial + +bus_name = net.connman +iface = net.connman.Manager +path = / +prop_name = State + +def check_state(state): + return state == 'ready' or state == 'online' + +def state_changed(mainloop, name, value): + if check_state(value): + mainloop.quit(); + +if __name__ == '__main__': + dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) + + bus = dbus.SystemBus() + + mainloop = gobject.MainLoop() + + bus.add_signal_receiver( + partial(state_changed, mainloop), + bus_name=bus_name, + dbus_interface=iface, + path=path, + signal_name=PropertyChanged, + arg0=prop_name) + + dev = bus.get_object(bus_name, path) + manager = dbus.Interface(dev, iface) + props = manager.GetProperties() + + if not check_state(props[prop_name]): + mainloop.run() -- 2.1.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/2] connman-wait-online.service.in: systemd service that waits for network
Create a systemd service that uses test/wait-for-ready to wait for any interface to be in the 'ready' or 'online' state. This service will be pulled by network-online.target. This is similar to NetworkManager-wait-online.service as provided by network manager and systemd-networkd-wait-online.service as provided by networkd. --- Makefile.am| 2 +- configure.ac | 3 ++- src/connman-wait-online.service.in | 12 3 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 src/connman-wait-online.service.in diff --git a/Makefile.am b/Makefile.am index af14dcc..3e216bf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -64,7 +64,7 @@ endif if SYSTEMD systemdunitdir = @SYSTEMD_UNITDIR@ -systemdunit_DATA = src/connman.service +systemdunit_DATA = src/connman.service src/connman-wait-online.service if VPN systemdunit_DATA += vpn/connman-vpn.service diff --git a/configure.ac b/configure.ac index 6f35c78..b4c1601 100644 --- a/configure.ac +++ b/configure.ac @@ -386,4 +386,5 @@ AM_CONDITIONAL(VPN, test ${enable_openconnect} != no -o \ AC_OUTPUT(Makefile include/version.h src/connman.service vpn/connman-vpn.service vpn/net.connman.vpn.service - scripts/connman connman.pc src/net.connman.service) + scripts/connman connman.pc src/net.connman.service + src/connman-wait-online.service) diff --git a/src/connman-wait-online.service.in b/src/connman-wait-online.service.in new file mode 100644 index 000..ff31496 --- /dev/null +++ b/src/connman-wait-online.service.in @@ -0,0 +1,12 @@ +[Unit] +Description=Wait for connman to have an interface ready +Requires=connman.service +After=connman.service +Before=network-online.target + +[Service] +Type=oneshot +ExecStart=@prefix@/lib/connman/test/wait-for-ready + +[Install] +WantedBy=network-online.target -- 2.1.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH] proxy: remove timeout on proxy cleanup
Make sure the timeout source is cleaned when proxy_lookup struct is deallocated. --- src/proxy.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/proxy.c b/src/proxy.c index 0d1a25a..f331de9 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -44,6 +44,9 @@ struct proxy_lookup { static void remove_lookup(struct proxy_lookup *lookup) { + if (lookup-watch 0) + g_source_remove(lookup-watch); + lookup_list = g_slist_remove(lookup_list, lookup); connman_service_unref(lookup-service); @@ -150,11 +153,6 @@ void connman_proxy_lookup_cancel(unsigned int token) } if (lookup) { - if (lookup-watch 0) { - g_source_remove(lookup-watch); - lookup-watch = 0; - } - if (lookup-proxy lookup-proxy-cancel_lookup) lookup-proxy-cancel_lookup(lookup-service, -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] agent: Check for pointer validity before cancelling agent request
Hi, On Wed, 2014-10-22 at 11:09 +0300, Hannu Mallat wrote: In the case where agent disappears from D-Bus while there are pending requests, request-driver may become null. As it's useless to send a Cancel to a nonexistent agent, not to mention accessing null pointer causes a crash, check for the pointer value before creating a D-Bus message. --- src/agent.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/agent.c b/src/agent.c index a340026..f67b30f 100644 --- a/src/agent.c +++ b/src/agent.c @@ -166,6 +166,9 @@ static int send_cancel_request(struct connman_agent *agent, { DBusMessage *message; + if (!request-driver) + return 0; + Mmmm, but the above can only happen in Jolla's version of ConnMan, there is/was commit 81195279ed07af08caf8c44fc390840b54b19ce2 in the Mer/Jolla tree that caused -driver to become NULL in some special circumstances. Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] agent: Check for pointer validity before cancelling agent request
On 22.10.2014 13:04, Patrik Flykt wrote: Mmmm, but the above can only happen in Jolla's version of ConnMan, there is/was commit 81195279ed07af08caf8c44fc390840b54b19ce2 in the Mer/Jolla tree that caused -driver to become NULL in some special circumstances. Ah, you're correct. Forgot about that when checking if I had something I hadn't upstreamed yet. BR, H. ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH v2] config: Handle IN_MOVED_TO inotify event
On Tue, 2014-10-21 at 15:59 +0300, Tomasz Bursztyka wrote: If the user moves a .config to /var/lib/connman which happens to be on the same fs, this will raise IN_MOVED_FROM/IN_MOVE_TO inotify events and not IN_DELETE/IN_CREATE. Applied, thanks! Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Bridging
Hello, Is it possible to bridge an interface that is managed by connman (e.g. wlan0) and an interface that is not managed by connman? Does connman provide something like: brctl addbr bridge brctl addif bridge wlan0 brctl addif bridge other Would it be possible to have connman manage the bridge? Thank you for your attention, and regards, Anton Voyl ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] Release references to technologies held by rfkill during cleanup.
On 10/22/2014 03:14 AM, Patrik Flykt wrote: So it seems ConnMan is not doing the technology_put() cleanup in the correct place. As you noticed this currently happens only in __connman_technology_remove(), and not on shutdown where only free_rfkill() is called. So the code starting with the line technology = technology_find(type); in __connman_technology_remove() should actually happen in free_rfkill() instead. That way no matter what frees up an rfkill structure also causes a technology_put() where the last one sends off a TechnologRemoved signal before destroying the technology structure. I tried this suggestion, but it does not fix the problem I was trying to fix. rfkill_list is destroyed in __connman_technology_cleanup which happens long after plugins are cleaned up. This means most if not all technologies have already been destroyed (possibly without being properly released/removed) before we get to this point. Based on your other reply though, it sounds like I need to fix my UI. I did not consider the case of a connman crash, which would be a good thing to handle. Also, there is the Released DBus signal that I was overlooking. Here is the patch of what I did to implement your suggestion. --- connman-1.26.orig/src/technology.c +++ connman-1.26/src/technology.c @@ -364,13 +364,6 @@ bool connman_technology_get_wifi_tetheri return true; } -static void free_rfkill(gpointer data) -{ -struct connman_rfkill *rfkill = data; - -g_free(rfkill); -} - static void technology_load(struct connman_technology *technology) { GKeyFile *keyfile; @@ -1764,18 +1757,26 @@ int __connman_technology_remove_rfkill(u g_hash_table_remove(rfkill_list, GINT_TO_POINTER(index)); -technology = technology_find(type); -if (!technology) -return -ENXIO; +return 0; +} -technology_apply_rfkill_change(technology, -technology-softblocked, !technology-hardblocked, false); +static void free_rfkill(gpointer data) +{ +struct connman_rfkill *rfkill = data; +struct connman_technology *technology; -technology_put(technology); +technology = technology_find(rfkill-type); +if (technology) { +technology_apply_rfkill_change(technology, +technology-softblocked, !technology-hardblocked, false); -return 0; +technology_put(technology); +} + +g_free(rfkill); } + int __connman_technology_init(void) { DBG(); ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH] Add support for PANU and GN roles to Bluetooth.
--- plugins/bluetooth.c | 42 +++--- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c index 82217d0..29e828d 100644 --- a/plugins/bluetooth.c +++ b/plugins/bluetooth.c @@ -36,7 +36,9 @@ #define BLUEZ_SERVICE org.bluez #define BLUEZ_PATH /org/bluez +#define BLUETOOTH_PAN_PANU 1115--1000-8000-00805f9b34fb #define BLUETOOTH_PAN_NAP 1116--1000-8000-00805f9b34fb +#define BLUETOOTH_PAN_GN 1117--1000-8000-00805f9b34fb #define BLUETOOTH_ADDR_LEN 6 @@ -50,6 +52,7 @@ struct bluetooth_pan { struct connman_network *network; GDBusProxy *btdevice_proxy; GDBusProxy *btnetwork_proxy; + const char *pan_role; }; static void address2ident(const char *address, char *ident) @@ -85,28 +88,32 @@ static bool proxy_get_bool(GDBusProxy *proxy, const char *property) return value; } -static bool proxy_get_nap(GDBusProxy *proxy) +static const char* proxy_get_role(GDBusProxy *proxy) { DBusMessageIter iter, value; if (!proxy) - return false; + return NULL; if (!g_dbus_proxy_get_property(proxy, UUIDs, iter)) -return false; + return NULL; dbus_message_iter_recurse(iter, value); while (dbus_message_iter_get_arg_type(value) == DBUS_TYPE_STRING) { const char *uuid; dbus_message_iter_get_basic(value, uuid); +if (strcmp(uuid, BLUETOOTH_PAN_PANU) == 0) + return panu; if (strcmp(uuid, BLUETOOTH_PAN_NAP) == 0) -return true; + return nap; +if (strcmp(uuid, BLUETOOTH_PAN_GN) == 0) + return gn; dbus_message_iter_next(value); } -return false; + return NULL; } static int bluetooth_pan_probe(struct connman_network *network) @@ -225,9 +232,11 @@ static void pan_connect_cb(DBusMessage *message, void *user_data) static void pan_connect_append(DBusMessageIter *iter, void *user_data) { - const char *role = BLUETOOTH_PAN_NAP; + const char *path = user_data; + struct bluetooth_pan *pan; - dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, role); + pan = g_hash_table_lookup(networks, path); + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, pan-pan_role); } static int bluetooth_pan_connect(struct connman_network *network) @@ -322,8 +331,10 @@ static void btnetwork_property_change(GDBusProxy *proxy, const char *name, static void pan_create_nap(struct bluetooth_pan *pan) { struct connman_device *device; + const char* role; - if (!proxy_get_nap(pan-btdevice_proxy)) { + role = proxy_get_role(pan-btdevice_proxy); + if (!role) { pan_remove_nap(pan); return; } @@ -366,6 +377,7 @@ static void pan_create_nap(struct bluetooth_pan *pan) connman_network_set_group(pan-network, ident); } + pan-pan_role = role; connman_device_add_network(device, pan-network); if (pan_connect(pan, NULL)) @@ -376,7 +388,7 @@ static void btdevice_property_change(GDBusProxy *proxy, const char *name, DBusMessageIter *iter, void *user_data) { struct bluetooth_pan *pan; - bool pan_nap = false; + const char* pan_role = NULL; if (strcmp(name, UUIDs) != 0) return; @@ -387,12 +399,12 @@ static void btdevice_property_change(GDBusProxy *proxy, const char *name, if (pan-network connman_network_get_device(pan-network)) - pan_nap = true; + pan_role = pan-pan_role; - DBG(network %p network nap %d proxy nap %d, pan-network, pan_nap, - proxy_get_nap(pan-btdevice_proxy)); + DBG(network %p network role %s proxy role %s, pan-network, pan_role, + proxy_get_role(pan-btdevice_proxy)); - if (proxy_get_nap(pan-btdevice_proxy) == pan_nap) + if (strcmp(proxy_get_role(pan-btdevice_proxy), pan_role) == 0) return; pan_create_nap(pan); @@ -447,7 +459,7 @@ static void pan_create(GDBusProxy *network_proxy) g_dbus_proxy_set_property_watch(pan-btdevice_proxy, btdevice_property_change, NULL); - DBG(pan %p %s nap %d, pan, path, proxy_get_nap(pan-btdevice_proxy)); + DBG(pan %p %s role %s, pan, path, proxy_get_role(pan-btdevice_proxy)); pan_create_nap(pan); } @@ -756,7 +768,7 @@ static void device_create(GDBusProxy *proxy) powered = proxy_get_bool(proxy, Powered); connman_device_set_powered(device, powered); - if (proxy_get_nap(proxy) !bluetooth_tethering) + if (proxy_get_role(proxy) != NULL