[PATCH v2] wpa_supplicant: Denitialize the driver if the last user went away

[Lubomir Rintel]

On Sat, 2014-11-01 at 17:08 +0200, Jouni Malinen wrote:
> On Thu, Oct 30, 2014 at 09:24:50AM +0100, Lubomir Rintel wrote:
> > It might be that the underlying infrastrucutre went away and the state is no
> > longer valid. We ought to reinitialize it once a device appears again.
> > 
> > This is the case when the nl80211 devices disappear and cfg8011 is remoed
> > afterwards. The netlink handle is no longer valid (returns ENOENT) and a new
> > one needs to be open if it's loaded back.
> 
> This is not the way the global_init/global_deinit was originally meant
> to be used, i.e., the assumption was more like this getting initialized
> once and then remain available even if no interfaces are enabled, e.g.,
> for get_interfaces() call to work.

I guess this is something specific to Windows NDIS drivers (only one
that seems to implement get_interfaces).

It seems to me that get_interfaces works whether the driver is
initialized or not; maybe it shouldn't even be passwd the global_priv
structure as it's unused anyway?

> > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> > @@ -3925,6 +3925,25 @@ static int wpa_supplicant_init_iface(struct wpa_supplicant *wpa_s,
> > +/* Deinitialize the driver if we're the last user. */
> > +static void wpa_drv_cleanup(struct wpa_supplicant *wpa_s)
> > +{
> > +	struct wpa_global *global = wpa_s->global;
> > +	struct wpa_supplicant *iface;
> > +	int i;
> > +
> > +	for (iface = global->ifaces; iface; iface = iface->next)
> > +		if (iface != wpa_s && iface->driver == wpa_s->driver)
> > +			return;
> > +
> > +	for (i = 0; wpa_drivers[i]; i++)
> > +		if (global->drv_priv[i] == wpa_s->global_drv_priv)
> > +			global->drv_priv[i] = NULL;
> > +
> > +	if (wpa_s->driver->global_deinit)
> 
> That would be a NULL pointer dereference if interface initialization
> fails early enough (wpa_s->driver == NULL in that case).

I see. Will fix.

> In addition to this, there is something wrong in how the global deinit
> gets handled, i.e., at least one more place can trigger a SIGSEGV when
> running through mac80211_hwsim test cases.

I've run the test suite. It runs now for hours, but I noticed no crashes
so far. However I see a lot of timeout and FAIL messages. I'm wondering
if some tests are expected to fail in master tree, or I need to fix my
setup:

[lkundrak at fedora20-2 hwsim]$ ./run-all.sh
START offchannel_tx_roc_gas 1/764
Exception: ANQP_GET command failed
FAIL offchannel_tx_roc_gas 0.283542 2014-11-11 13:28:00.406416
...
START p2p_device_group_remove 420/764
Exception: [Errno 111] Connection refused
FAIL p2p_device_group_remove 0.104766 2014-11-11 13:58:50.080920
...

> As such, I cannot really
> apply this until those issues have been found and fixed. Then again, I'm
> not sure whether this is really the best way of handling the issue which
> I'm assuming is the main reason for this patch, i.e., need to
> re-initialize some socket for the case of cfg80211 getting reloaded.

Correct. What would be the other possible fix? Address that in kernel?

Lubo