[PATCH 1/1] Avoid double invocation of wpa_driver_nl80211_sta_remove function from ap_sta_disconnect context.

[Jouni Malinen]

On Tue, Aug 16, 2016 at 10:43:44AM +0530, Jithu Jance wrote:
> For "device_ap_sme" devices, the ap_sta_disconnect call in
> supplicant results in two calls to wpa_driver_nl80211_sta_remove.
> 
> ap_sta_disconnect > hostapd_drv_sta_deauth > wpa_driver_nl80211_sta_remove
> ap_sta_disconnect > ap_sta_deauth_cb_timeout > ap_sta_remove > hostapd_drv_sta_remove
> 
> The ap_sta_deauth_cb_timeout is invoked immediately (timeout of [0,0]) for
> device_ap_sme devices. The hostapd_drv_sta_deauth call can be avoided
> for devices without WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS set.

Hmm.. That does indeed seem to be the case, but it should be noted that
there is a difference in calling hostapd_drv_sta_remove() and
hostapd_drv_sta_deauth(): the former does not pass the reason code to
the driver wrapper while the latter does.

> diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c
> @@ -1198,7 +1198,8 @@ void ap_sta_disconnect(struct hostapd_data *hapd, struct sta_info *sta,
>  	if (sta == NULL && addr)
>  		sta = ap_get_sta(hapd, addr);
>  
> -	if (addr)
> +	if (addr && (hapd->iface->drv_flags &
> +			WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS))
>  		hostapd_drv_sta_deauth(hapd, addr, reason);
>  
>  	if (sta == NULL)

This is the former case and if this is made conditional, all drivers
that do not set WPA_DRIVER_FLAGS_DEAUTH_TX_STATUS would lose the reason
code when they get only the second call from ap_sta_remove(). I don't
think this is acceptable.

In addition, the sta == NULL case would return from ap_sta_disconnect()
without even registering the ap_sta_disassoc_cb_timeout() callback at
all. That does not sound correct either, i.e., this condition on
skipping the hostapd_drv_sta_deauth() call should likely apply only if
sta != NULL.

For the reason code disappearing issue, one could consider extending
hostapd_drv_sta_remove() support passing a reason code to the driver,
but I'm not really sure this is the correct thing to do.. In other
words, I think I'd rather leave this as-is.

Other than debug logs showing some warnings, are there any real issues
noticeable by external devices that this patch is fixing?
 
-- 
Jouni Malinen                                            PGP id EFC895FA