[PATCH] gsupplicant: Null check DBusMessageIter to avoid abort
Depending on how dbus is built it might abort when passing null pointers to it.
This patch checks an iter while parsing dbus errors, the iter might be NULL,
for instance when an operation is aborted.
This trace shows the issue:
0 0xb6c24144 in __GI_raise (sig=sig@entry=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:56
1 0xb6c27c0c in __GI_abort () at abort.c:89
2 0xb6e73ca8 in _dbus_abort () at
/work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-sysdeps.c:94
3 0xb6e6b1b8 in _dbus_warn_check_failed (format=0xb6e77d44 dbus message
iterator is NULL\n) at
/work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-internals.c:290
4 0xb6e5a728 in _dbus_message_iter_check (iter=0x0) at
/work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-message.c:727
5 0xb6e5b734 in dbus_message_iter_get_arg_type (iter=iter@entry=0x0) at
/work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-message.c:2065
6 0x00025398 in parse_supplicant_error (iter=iter@entry=0x0) at
gsupplicant/supplicant.c:3799
7 0x00025430 in interface_add_network_result (error=optimized out, iter=0x0,
user_data=0x175b620) at gsupplicant/supplicant.c:3878
8 0x0002adac in supplicant_dbus_method_call_cancel_all
(caller=caller@entry=0x1739d98) at gsupplicant/dbus.c:445
9 0x00029480 in g_supplicant_interface_cancel
(interface=interface@entry=0x1739d98) at gsupplicant/supplicant.c:3020
10 0x00029898 in g_supplicant_interface_remove (interface=0x1739d98,
callback=callback@entry=0x0, user_data=user_data@entry=0x0) at
gsupplicant/supplicant.c:3487
11 0x00020df0 in wifi_disable (device=0x17306b0) at plugins/wifi.c:1134
12 0x0002f94c in __connman_device_disable (device=0x17306b0) at src/device.c:248
13 0x0005d450 in technology_affect_devices (enable_device=optimized out,
technology=optimized out, technology=optimized out) at src/technology.c:630
14 0x0005d4d8 in technology_disable (technology=0x1738600) at
src/technology.c:780
15 0x0005e634 in set_powered (powered=false, msg=0x1732aa0,
technology=0x1738600) at src/technology.c:802
16 set_property (conn=optimized out, msg=0x1732aa0, data=0x1738600) at
src/technology.c:937
17 0x00077878 in process_message (connection=connection@entry=0x1730440,
message=message@entry=0x1732aa0, iface_user_data=0x1738600,
iface_user_data@entry=0x1, method=0x8610c technology_methods+24,
method=0x8610c technology_methods+24)
at gdbus/object.c:259
18 0x00077cb8 in generic_message (connection=0x1730440,
message=message@entry=0x1732aa0, user_data=user_data@entry=0x173b248) at
gdbus/object.c:1070
19 0xb6e61260 in _dbus_object_tree_dispatch_and_unlock (tree=0x17301f8,
message=message@entry=0x1732aa0, found_object=found_object@entry=0xbec56bbc)
at /work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-object-tree.c:862
20 0xb6e51ad0 in dbus_connection_dispatch
(connection=connection@entry=0x1730440) at
/work/my-arch/dbus/1.6.18-r0/dbus-1.6.18/dbus/dbus-connection.c:4672
21 0x00074404 in message_dispatch (data=0x1730440) at gdbus/mainloop.c:72
22 0xb6ebf870 in g_idle_dispatch (source=optimized out, callback=0x743f4
message_dispatch, user_data=optimized out)
at /work/my-arch/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/gmain.c:5251
23 0xb6ec2bd4 in g_main_dispatch (context=0x172e028) at
/work/my-arch/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/gmain.c:3066
24 g_main_context_dispatch (context=context@entry=0x172e028) at
/work/my-arch/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/gmain.c:3642
25 0xb6ec2f60 in g_main_context_iterate (context=0x172e028,
block=block@entry=1, dispatch=dispatch@entry=1, self=optimized out)
at /work/my-arch/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/gmain.c:3713
26 0xb6ec3430 in g_main_loop_run (loop=0x172e1e8) at
/work/my-arch/glib-2.0/1_2.38.2-r0/glib-2.38.2/glib/gmain.c:3907
27 0x000148f8 in main (argc=496656, argv=0x7cc18) at src/main.c:688
---
gsupplicant/supplicant.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 534944b..8c1db01 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -3796,7 +3796,8 @@ static int parse_supplicant_error(DBusMessageIter *iter)
* invalid message format but this error should be interpreted as
* invalid-key.
*/
- while (dbus_message_iter_get_arg_type(iter) == DBUS_TYPE_STRING) {
+ while (iter
+ dbus_message_iter_get_arg_type(iter) == DBUS_TYPE_STRING) {
dbus_message_iter_get_basic(iter, key);
if (strncmp(key, psk, 3) == 0 ||
strncmp(key, wep_key, 7) == 0 ||
--
1.9.1
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network
___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] dhcp: Fixed Crash on Switching Network
Hi, We still don't see your patch unfortunately. Can you try with git send-email? It's an opens-source mailing list here, so you don't need to send any header specifying there is trade secret etc... Thanks, Tomasz ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network
0001-dhcp-Fixed-Crash-on-Switching-Network.patch Description: Binary data ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH] dhcp: Fixed Crash on Switching Network
Hi,
Sometimes while switching network dhcp_initialize() fails because
interface is not up and hence dhcp-dhcp_client remains NULL. Here we
don't check return type of dhcp_initialize() and go on to call function
g_dhcp_client_start() and crash occurs.
Good catch, but this makes me think there is another bug: if the device
is down,
connman should not even think of starting dhcp on it, so there might be
another issue
to get fixed here. What version of connman are you using btw?
Below trace is obtained when connman crashes:
connmand[19034]: Aborting (signal 11) [/usr/local/sbin/connmand]
connmand[19034]: backtrace
connmand[19034]: #0 0xb7630f38 in /lib/i386-linux-gnu/libpthread.so.0
connmand[19034]: #1 0x8055a22 in debug() at client.c:0
connmand[19034]: #2 0x8058837 in g_dhcp_client_start() at polkit.c:0
connmand[19034]: #3 0x80a4772 in __connman_dhcp_start() at polkit.c:0
connmand[19034]: #4 0x8082a80 in set_connected.part.8() at network.c:0
connmand[19034]: #5 0x8082f7f in connman_network_set_connected() at
??:0
connmand[19034]: #6 0x805f921 in eth_network_connect() at ethernet.c:0
connmand[19034]: #7 0x8082dc3 in __connman_network_connect() at
polkit.c:0
connmand[19034]: #8 0x808e7e4 in __connman_service_connect() at
polkit.c:0
connmand[19034]: #9 0x808eef0 in auto_connect_service() at service.c:0
connmand[19034]: #10 0x808efde in run_auto_connect() at service.c:0
connmand[19034]: #11 0xb76cea3f in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #12 0xb76cdd46 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #13 0xb76ce0e5 in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #14 0xb76ce52b in /lib/i386-linux-gnu/libglib-2.0.so.0
connmand[19034]: #15 0x80544cd in main() at polkit.c:0
connmand[19034]: #16 0xb739b4d3 in /lib/i386-linux-gnu/libc.so.6
connmand[19034]: +++
---
src/dhcp.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/dhcp.c b/src/dhcp.c
index 132b787..a2780ca 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -587,6 +587,7 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
{
const char *last_addr = NULL;
struct connman_dhcp *dhcp;
+ int err;
DBG();
@@ -617,7 +618,9 @@ int __connman_dhcp_start(struct connman_ipconfig *ipconfig,
g_hash_table_insert(ipconfig_table, ipconfig, dhcp);
- dhcp_initialize(dhcp);
+ err = dhcp_initialize(dhcp);
+ if(err != 0)
+ return err;
}
There is an issue here: dhcp structure is inserted before hand in the
hash table but can
be uninitialized in case of the error you spotted.
So I would instead change the order here: run first dhcp_initialize
- if an error is returned, free the dhcp structure, and return the error
code.
- on success: insert it in the hash table.
Thanks,
Tomasz
___
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman
Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network
___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: Re: [PATCH] dhcp: Fixed Crash on Switching Network
___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [RFC] Exposing network credentials to an application
Hi Patrik, There's a chicken-and-egg problem in the use case Grant mentioned which unfortunately can't be solved with .config file passing. In the presented use case, the user selects and authenticates to a network when they set up the first device. The application instructs connman to connect to the specified network with the given credentials via the services API. Once a connection is made, connman saves its configuration for this network somewhere in /var/lib/connman/wifi_*/, but ***does not*** create a .config file with this information. At this point, the application on the first device can't produce a .config file for the second device. It can't copy one out of /var/log/connman/, as there is no .config file to copy. It can't extract all information necessary to build the file from scratch via the services API, as the WiFi PSK is obscured. To extract the PSK in this scenario, the application would need to root around in /var/lib/connman/wifi_*/, as Grant observed. One option is for the application on the first device to create a .config file based on the user's network credentials, instead of passing this information to connman via the services API. This seems like a poor solution, however, as the logic to create .config files is then duplicated between connman and the application. Also, any application which may need to retrieve a service's PSK in the future is forced to use file I/O rather than the services API to inform connman about preferred services. Also note that the second device might not even run connman (or Linux, for that matter). This means that to provision the second device, if the WiFi PSK is obscured via the services API, and if the application creates a .config file based on the user's input, then the application needs logic to parse .config files as well as create them. At this point, the application is interacting with connman more through file I/O than through D-Bus. It would be easier for the application (and application developers :)) if all necessary information to provision a new device could be retrieved from connman via the services API. What do you think? Thanks, Drew On Mon, Jul 28, 2014 at 5:14 AM, Patrik Flykt patrik.fl...@linux.intel.com wrote: Hi, On Fri, 2014-06-27 at 12:27 -0700, Grant Erickson wrote: Imagine another system, different from a mobile device, perhaps like the first that can communicate and authenticate with it out-of-band, based on some provided user authority. Over that channel, the first device would like to tell the second, When you encounter this WiFi network, you may connect to it with these credentials.. This can be achieved with a properly set up .config file obtained over the out-of-band channel. The .config file format is described in doc/config-format.txt. While you're correct that one could certainly root about int he /var/lib/connman directory and pull this, such a mechanism will not The .config files are the only ones in /var/lib/connman that users can modify. As they are documented, they belong to the API and are not going to change. Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
