[PATCH] nl80211: Clear preq NL handle after Unsbcsribe mgmt

Jouni Malinen j at w1.fi
Sat Dec 11 08:27:58 PST 2021


On Mon, Nov 22, 2021 at 12:04:12PM +0800, Ouden Lin wrote:
> At the start of GO (start AP), Unsbcsribe mgmt will delete all management reports, including Probe Requst,
> However, nl_preq is still not deleted.

Well, it is for the drv->device_ap_sme == 0 case at the beginning of
nl80211_setup_ap()..

> Finally, Go cannot enable probe request reporting until the trigger is disabled.
> 
> This patch will check nl_preq after Unsubscribe mgmt.

Hmm.. It looks like something would indeed be needed here, but this
looks a bit strange with the proposed change.

> nl80211: Setup AP operations for P2P group (GO)
> nl80211: Unsubscribe mgmt frames handle 0x8888dea1bcc6c2c9 (start AP)
> nl80211: Setup AP(wlan1) - device_ap_sme=1 use_monitor=0
> nl80211: Subscribe to mgmt frames with AP handle 0x5629344e4a40 (device SME)

In the device_ap_sme=0 case, there is already a "Disable Probe Request
reporting.." between those last two lines..

> nl80211: Probe Request reporting already on! nl_preq=0x8888dea1bcdcbca9

So this does not happen. Or well, there is not even an attempt to enable
this at all. In other words, the commit message should be clearer on
this being an issue only for the AP SME in the driver case.

> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
>  	if (is_ap_interface(nlmode)) {
>  		nl80211_mgmt_unsubscribe(bss, "start AP");
> +		if (bss->nl_preq) {
> +			wpa_printf(MSG_DEBUG, "nl80211: Disable Probe Request "
> +				   "reporting nl_preq=%p", bss->nl_preq);
> +			nl80211_destroy_eloop_handle(&bss->nl_preq, 0);
> +		}

So this would end up unmasking the bss->nl_preq pointer, unregistering
the eloop handler for the socket, and deleting that unmasked
bss->nl_preq pointer while still leaving bss->nl_preq to point to that
now freed handle.

Is it clear that this really works in all cases? What wuld happen if
wpa_driver_nl80211_deinit() were to call
wpa_driver_nl80211_probe_req_report(bss, 0) after this? Wouldn't that
end up dereferencing an invalid pointer?

-- 
Jouni Malinen                                            PGP id EFC895FA



More information about the Hostap mailing list