Segmentation Fault when removing P2P group interface on an embedded ARM system

[Jouni Malinen]

On Mon, Aug 05, 2013 at 10:21:05AM +0900, ???? wrote:
> On an embedded ARM system(with Kernel is 3.10) I have issues(segmentation
> fault) to remove P2P group interface. 
> 
> The test sequences are as follows:
> 
> 1. Run the wpa_supplicant daemon:  wpa_supplicant -Dnl80211 -iwlan0 -c
> p2p.conf -u -dd
> 
> 2. Create the P2P GO : wpa_cli -iwlan0 p2p_group_add 
> 3. Remove P2P group interface : wpa_cli -iwlan0 p2p_group_remove p2p-wlan0-0

> wlan0: P2P-GROUP-REMOVED p2p-wlan0-0 GO reason=REQUESTED
> dbus: Unregister network object
> '/fi/w1/wpa_supplicant1/Interfaces/3/Networks/0'
> dbus: wpa_dbus_unregister_object_per_iface: Could not obtain object's
> private data: /fi/w1/wpa_supplicant1/Interfaces/3/Networks/0
> Segmentation fault

Thanks for reporting this. This did not result in a segmentation fault
in my test run now with Ubuntu 12.04 (which you seemed to have pointed
out below, too), but anyway, yes, this should not really happen since
D-Bus is quite unhappy about attempts to remove not existing objects
(i.e., it can kill the process at least with some versions).

> I debugged it and I found the problem.
> 
> It is to unregister Network's path which is not object path.
> 
> I can see the obect path related P2P after creating the P2P GO
> 
> - /fi/w1/wpa_supplicant1/Interfaces/3/
> 
> It is the object path related to p2p-wlan0-0 and
> /fi/w1/wpa_supplicant1/Interfaces/3/Networks/0 is not object path. 

Based on wpas_notify_network_added() implementation, it looks like there
was an attempt to not register P2P network blocks in D-Bus. However,
this was not really behaving correctly in all cases. This autonomous GO
with p2p_group_add was one such case. I fixed this by checking
ssid->p2p_group to skip P2P groups more consistently in
wpas_notify_netowrk_{added,removed}.

> In wpa_dbus_unregister_object_per_iface() in dbus_new_helper.c,
> if (!dbus_connection_unregister_object_path(con, path))       ->
> Segmentation fault point

This is the part I do not exactly like about some libdbus versions..
Anyway, yes, we need to be careful about not trying to unregister
objects that do not exist.

> Why do '/fi/w1/wpa_supplicant1/Interfaces/3/Networks/0' unregister ? 
> 
> As I mention before, it is not object path.

That's because the inconsistent network added/removed behavior I
mentioned above.. This case should not have tried to remove the object.

> In x86 system(ubuntu 12.04), there are not segmentation fault and it works
> fine however I can see unknown message which is not created by
> wpa_supplicant as bellow:

There are some version of libdbus, where I've seen similar cases force
the process to exit even if not with segmentation fault.

> I think if the value of path is not object path, it is not necessary to do
> dbus_connection_unregister_object_path(con, path).

Correct. Though, we should not even have tried to call
wpa_dbus_unregister_object_per_iface() in this specific case.

> I changed the source code and both system work fine.
> 
> diff -urN A/wpa_supplicant/dbus/dbus_new_helpers.c
> B/wpa_supplicant/dbus/dbus_new_helpers.c
>         if (!obj_desc) {
>                 wpa_printf(MSG_ERROR, "dbus: %s: Could not obtain object's "
>                            "private data: %s", __func__, path);
> +               return 0;
>         } else {
>                 eloop_cancel_timeout(flush_object_timeout_handler, con,
>                                      obj_desc); 

I fixed this in a bit different way to be consistent with other
functions in dbus_new_helpers.c, i.e., by getting rid of the else branch
(no need for it since the return statement in !obj_desc case).

-- 
Jouni Malinen                                            PGP id EFC895FA