[PATCH v3 2/3] P2P: optimize join scan freq

[Jouni Malinen]

On Tue, Dec 20, 2022 at 07:15:46PM -0800, Matthew Wang wrote:
> Allow clients to force the BSSID of an auto GO. If the auto GO has been
> discovered on another interface, optimize scan frequency by performing
> a single channel scan first. Android and ChromeOS use this to streamline
> auto GO discovery.

How would the P2P Client know which BSSID the GO is using in this
instance? The BSSID, i.e., the P2P Interface Address, may change between
each invocation of a persistent group.

> diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c
> @@ -240,7 +240,7 @@ void wpa_bss_remove(struct wpa_supplicant *wpa_s, struct wpa_bss *bss,
> - * @bssid: BSSID
> + * @bssid: BSSID, or %NULL to match any BSSID

> @@ -252,7 +252,8 @@ struct wpa_bss * wpa_bss_get(struct wpa_supplicant *wpa_s, const u8 *bssid,
>  	if (!wpa_supplicant_filter_bssid_match(wpa_s, bssid))
>  		return NULL;

That could result in NULL pointer dereferencing the bssid parameter,
i.e., this call needs to be made conditional on bssid being not-NULL.

> diff --git a/wpa_supplicant/dbus/dbus_new_handlers_p2p.c b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
> +++ b/wpa_supplicant/dbus/dbus_new_handlers_p2p.c
> @@ -356,6 +356,7 @@ DBusMessage * wpas_dbus_handler_p2p_group_add(DBusMessage *message,
> +	int force_go_bssid = 0;

That should really be bool/false here.

> @@ -382,6 +383,9 @@ DBusMessage * wpas_dbus_handler_p2p_group_add(DBusMessage *message,
> +		} else if (os_strcmp(entry.key, "force_go_bssid") &&

os_strcmp() returns 0 on match, i.e., this needs " == 0" to be added to
work correctly.

> +			   entry.type == DBUS_TYPE_BOOLEAN) {
> +			force_go_bssid = entry.bool_value;

>  		if (wpas_p2p_group_add_persistent(wpa_s, ssid, 0, freq, 0, 0, 0,
>  						  0, 0, 0, 0, NULL, 0, 0,
> -						  false, retry_limit)) {
> +						  false, retry_limit, true)) {

true? Was that supposed to be force_go_bssid instead?

> diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c

>  static int wpas_start_p2p_client(struct wpa_supplicant *wpa_s,
>  				 struct wpa_ssid *params, int addr_allocated,
> -				 int freq, int force_scan, int retry_limit)
> +				 int freq, int force_scan, int retry_limit,
> +				 bool force_go_bssid)

> +	if (force_go_bssid && params->bssid_set) {
> +		ssid->bssid_set = 1;
> +		os_memcpy(ssid->bssid, params->bssid, ETH_ALEN);
> +	}

The params->bssid here is the bssid value in the special network profile
that is used to store the persistent group information. That special
case uses the bssid value to store the P2P Device Address of the GO; not
the P2P Interface Address (which would be the BSSID). How is this
supposed to work if the GO uses P2P Interface Addresses that differ from
the P2P Device Address?

-- 
Jouni Malinen                                            PGP id EFC895FA