[PATCH] wpa_supplicant: Set null radio when no more used

[Eduardo Abinader]

Hi Jouni,

Please see coments below.

On Fri, Feb 6, 2015 at 4:22 PM, Jouni Malinen <j at w1.fi> wrote:
> On Fri, Feb 06, 2015 at 09:44:27AM -0400, Eduardo Abinader wrote:
>> This patch avoids a segmentation fault that is occurring
>> when a removed pending p2p interface receives a delayed
>> scan result and crashes when handled in
>> wpa_supplicant_event_scan_results.
>>
>> The scenario being used is a request to pbc join an
>> autonomous p2p group.
>
> Would you have a debug log showing this crash? I find it a bit difficult
> to see why this would happen for two reasons: 1) the wpa_s instance is
> freed immediately after this call (i.e., there is no way a scan results
> could be processed for the interface) and 2) driver_nl80211 drops the
> scan result indication for the interface once it has been removed.
>

Log file is here:

http://filebin.ca/1ql0zqDLMH1y/wpas_crash.log.gz

>> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
>> @@ -3675,13 +3675,13 @@ static void radio_remove_interface(struct wpa_supplicant *wpa_s)
>> -     wpa_s->radio = NULL;
>>       if (!dl_list_empty(&radio->ifaces))
>>               return; /* Interfaces remain for this radio */
>>
>>       wpa_printf(MSG_DEBUG, "Remove radio %s", radio->name);
>>       eloop_cancel_timeout(radio_start_next_work, radio, NULL);
>>       os_free(radio);
>> +     wpa_s->radio = NULL;
>
> This radio_remove_interface() function is called only from
> wpa_supplicant_deinit_iface() and this call is followed by the driver
> interface deinit and os_free(wpa_s) before returning to eloop. In other
> words, there is no way this specific wpa_s instance could be used in
> wpa_supplicant_event_scan_results() after this unless there are some
> much more severe issues of continuing to use freed memory.
>
> Allowing wpa_s->radio to continue to point to the radio in the
> other-interfaces-remain case looks a bit suspicious since if there were
> actual real uses of the wpa_s instance after this, I don't see what
> would prevent that other remaining wpa_s instance from getting remove
> and deleting the radio instance. After that, the wpa_s->radio on the
> not-cleared-to-NULL interface would point to freed memory. Anyway, this
> path should obviously not happen either due to that os_free(wpa_s)
> following this call..

Actually wpa_s->radio is only being freed, when no other interface is
using it, like
wlanX. The patches only ensures radio is to null, when freed.

>
> --
> Jouni Malinen                                            PGP id EFC895FA
> _______________________________________________
> HostAP mailing list
> HostAP at lists.shmoo.com
> http://lists.shmoo.com/mailman/listinfo/hostap