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

[Jouni Malinen]

On Fri, Feb 06, 2015 at 04:45:21PM -0400, Eduardo Abinader wrote:
> Log file is here:
> 
> http://filebin.ca/1ql0zqDLMH1y/wpas_crash.log.gz

Thanks! I was able to reproduce this and confirm that this is indeed due
to use of freed memory after the P2P group interface gets removed.

> >> 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.

It doesn't do that. It ensures that wpa_s->radio is _not_ set to NULL
when the radio instance is not freed, but it in no way ensures that
wpa_s->radio gets set to NULL (for all wpa_s instances having used the
radio) when the radio gets removed. That's the part that I don't think
is appropriate since it could leave behind references to a freed radio
on other interfaces than the one that removed the radio.

Anyway, this change is not sufficient to address the issue even if it
happens to hide the crash in some cases. The wpa_s instance is already
freed when wpa_s->radio is used on the scan result handling part. I
fixed this issue by skipping the processing steps following removal of
the P2P group interface:
http://w1.fi/cgit/hostap/commit/?id=b0e669beebbb0d764c354f6ef7736c58f82681ec

-- 
Jouni Malinen                                            PGP id EFC895FA