[PATCH] Use default IEs in wpa_supplicant_trigger_scan

[Jouni Malinen]

On Wed, Mar 31, 2021 at 11:52:21AM -0700, Matthew Wang wrote:
> wpa_supplicant_trigger_scan previously wouldn't include any of the IEs
> generated by wpa_supplicant_extra_ies. Instruct it to do so in most
> cases. This is necessary because MBO STAs are required to include MBO
> capabilities in their probe requests.

> diff --git a/wpa_supplicant/scan.c b/wpa_supplicant/scan.c
> @@ -278,19 +278,40 @@ static void wpas_trigger_scan_cb(struct wpa_radio_work *work, int deinit)
>   * wpa_supplicant_trigger_scan - Request driver to start a scan
>   * @wpa_s: Pointer to wpa_supplicant data
>   * @params: Scan parameters
> + * @default_ies: Whether or not to use the default IEs in the probe request.
> + * Note that this will free any existing IEs set in @params, so this shouldn't
> + * be set if the IEs have already been set with wpa_supplicant_extra_ies.
> + * Otherwise, wpabuf_free will lead to a double-free.

This sounds a bit scary when most existing calls to
wpa_supplicant_trigger_scan() were modified to use default_ies == true.

> +	if (default_ies) {
> +		if (params->extra_ies_len) {
> +			os_free((u8 *) params->extra_ies);
> +		}

How can this be sure that params->extra_ies (const u8 pointer for a
reason to imply it might not be allocated from heap) is actually
allocated with os_malloc/zalloc() and something that is not referenced
from somewhere else? Should this set params->extra_ies to NULL after it
was freed?

> +		ies = wpa_supplicant_extra_ies(wpa_s);
> +		if (ies) {
> +			params->extra_ies = wpabuf_head(ies);
> +			params->extra_ies_len = wpabuf_len(ies);
> +		}

This would seem to leave params->extra_ies_len > 0 and params->extra_ies
pointing to freed memory if that wpa_supplicant_extra_ies() call fails..

-- 
Jouni Malinen                                            PGP id EFC895FA