[PATCH v1 0/3] wifi: brcm80211: avoid memcpy warning when CONFIG_FORTIFY_SOURCE=y

[PATCH v1 0/3] wifi: brcm80211: avoid memcpy warning when CONFIG_FORTIFY_SOURCE=y

[Petr Tesarik]

From: Petr Tesarik <petr.tesarik.ext@huawei.com>

When built with CONFIG_FORTIFY_SOURCE=y, the brcmfmac module triggers a
memcpy() warning like this:

 ------------[ cut here ]------------
 memcpy: detected field-spanning write (size 76) of single field "&params_le->channel_list[0]" at drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 (size 2)
 WARNING: CPU: 2 PID: 991 at drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
 Modules linked in: qrtr(E) algif_hash(E) aes_neon_bs(E) aes_neon_blk(E) algif_skcipher(E) af_alg(E) bnep(E) brcmfmac_wcc(E) bcm2835_v4l2(CE) bcm2835_mmal_vchiq(CE) videobuf2_vmalloc(E) videobuf2_memops(E) videobuf2_v4l2(E) videodev(E) hci_uart(E) btsdio(E) videobuf2_common(E) btbcm(E) mc(E) snd_bcm2835(CE) bluetooth(E) snd_pcm(E) brcmfmac(E) snd_timer(E) cpufreq_dt(E) snd(E) soundcore(E) cfg80211(E) ecdh_generic(E) brcmutil(E) raspberrypi_cpufreq(E) rfkill(E) vchiq(CE) bcm2711_thermal(E) leds_gpio(E) fuse(E) efi_pstore(E) dmi_sysfs(E) ip_tables(E) x_tables(E) rpcsec_gss_krb5(E) auth_rpcgss(E) nfsv4(E) dns_resolver(E) nfs(E) lockd(E) grace(E) fscache(E) netfs(E) af_packet(E) mmc_block(E) xhci_pci(E) xhci_pci_renesas(E) xhci_hcd(E) usbcore(E) usb_common(E) clk_raspberrypi(E) gpio_raspberrypi_exp(E) bcm2835_dma(E) crct10dif_ce(E) virt_dma(E) pcie_brcmstb(E) sdhci_iproc(E) gpio_regulator(E) sdhci_pltfm(E) sdhci(E) mmc_core(E) fixed(E) nvmem_rmem(E) sunrpc(E) sg(E) dm_multipath(E) dm_mod(E) efivarfs(E)
 Unloaded tainted modules: aes_ce_cipher(E):1
 CPU: 2 PID: 991 Comm: wpa_supplicant Tainted: G         C  E      6.5.0-rc4-dynswiotlb+ #27 2ec0961165cc91fdbec101d9d43b3331ba4f0927
 Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.04 04/01/2023
 pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
 lr : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
 sp : ffff8000829ab590
 x29: ffff8000829ab590 x28: 0000000000000000 x27: 0000000000000001
 x26: ffff000105e7e0a4 x25: ffff00010a0bcb48 x24: ffff000101e03800
 x23: ffff000105ec8920 x22: ffff000106332980 x21: ffff00010a0bc0c0
 x20: ffff00010a0bcb90 x19: ffff00010a0bc108 x18: ffffffffffffffff
 x17: 0000000000000000 x16: 0000000000000000 x15: 616f72622f737365
 x14: 6c657269772f7465 x13: 616d666d6372622f x12: 31313230386d6372
 x11: 00000000ffffdfff x10: ffff800081ad3328 x9 : ffff800080130694
 x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
 x5 : ffff0001fef75e00 x4 : 0000000000000000 x3 : 0000000000000027
 x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00010aa0e000
 Call trace:
  brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
  brcmf_run_escan+0x148/0x1a0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
  brcmf_do_escan+0x74/0xe0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
  brcmf_cfg80211_scan+0xcc/0x298 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
  rdev_scan+0x38/0x158 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
  cfg80211_scan+0x134/0x178 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
  nl80211_trigger_scan+0x3e8/0x768 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
  genl_family_rcv_msg_doit.isra.0+0xc0/0x130
  genl_rcv_msg+0x1e4/0x278
  netlink_rcv_skb+0x64/0x138
  genl_rcv+0x40/0x60
  netlink_unicast+0x1cc/0x2d8
  netlink_sendmsg+0x1d4/0x448
  sock_sendmsg+0x64/0xc0
  ____sys_sendmsg+0x260/0x2e0
  ___sys_sendmsg+0x88/0xf0
  __sys_sendmsg+0x70/0xd8
  __arm64_sys_sendmsg+0x2c/0x40
  invoke_syscall+0x78/0x100
  el0_svc_common.constprop.0+0x100/0x130
  do_el0_svc+0x40/0xa8
  el0_svc+0x34/0x138
  el0t_64_sync_handler+0x120/0x130
  el0t_64_sync+0x1a8/0x1b0
 ---[ end trace 0000000000000000 ]---

This is in fact a false positive, but the data structures can be improved
to make the checker happy.

Tested on a Raspberry Pi 4 by running scans in a loop while activating and
deactivating the wireless connection in parallel to cause scan aborts.

Petr Tesarik (3):
  wifi: brcm80211: drop struct brcmf_p2p_scan_le
  wifi: brcm80211: separate abort scan param prepare from actual scan
  wifi: brcm80211: change channel_list to a flexible array

 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 67 ++++++++++---------
 .../broadcom/brcm80211/brcmfmac/fwil_types.h  | 14 +++-
 .../broadcom/brcm80211/brcmfmac/p2p.c         | 38 +++--------
 3 files changed, 57 insertions(+), 62 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

[Petr Tesarik]

From: Petr Tesarik <petr.tesarik.ext@huawei.com>

The 'S' scan method is not implemented. The type member is always written
as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
replaced with struct brcmf_escan_params_le.

Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 .../broadcom/brcm80211/brcmfmac/p2p.c         | 38 ++++---------------
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index d4492d02e4ea..915f95760470 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
 	WL_P2P_DISC_ST_SEARCH
 };
 
-/**
- * struct brcmf_p2p_scan_le - P2P specific scan request.
- *
- * @type: type of scan method requested (values: 'E' or 'S').
- * @reserved: reserved (ignored).
- * @eparams: parameters used for type 'E'.
- * @sparams: parameters used for type 'S'.
- */
-struct brcmf_p2p_scan_le {
-	u8 type;
-	u8 reserved[3];
-	union {
-		struct brcmf_escan_params_le eparams;
-		struct brcmf_scan_params_le sparams;
-	};
-};
-
 /**
  * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
  *
@@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
 {
 	struct brcmf_pub *drvr = p2p->cfg->pub;
 	s32 ret = 0;
-	s32 memsize = offsetof(struct brcmf_p2p_scan_le,
-			       eparams.params_le.channel_list);
+	s32 memsize = offsetof(struct brcmf_escan_params_le,
+			       params_le.channel_list);
 	s32 nprobes;
 	s32 active;
 	u32 i;
 	u8 *memblk;
 	struct brcmf_cfg80211_vif *vif;
-	struct brcmf_p2p_scan_le *p2p_params;
+	struct brcmf_escan_params_le *eparams;
 	struct brcmf_scan_params_le *sparams;
 
 	memsize += num_chans * sizeof(__le16);
@@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
 		ret = -EINVAL;
 		goto exit;
 	}
-	p2p_params = (struct brcmf_p2p_scan_le *)memblk;
-	sparams = &p2p_params->eparams.params_le;
+	eparams = (struct brcmf_escan_params_le *)memblk;
+	sparams = &eparams->params_le;
 
 	switch (search_state) {
 	case WL_P2P_DISC_ST_SEARCH:
@@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
 
 	brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
 
-	/*
-	 * set p2p scan parameters.
-	 */
-	p2p_params->type = 'E';
-
 	/* determine the scan engine parameters */
 	sparams->bss_type = DOT11_BSSTYPE_ANY;
 	sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
@@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
 		sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
 
 	/* set the escan specific parameters */
-	p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
-	p2p_params->eparams.action =  cpu_to_le16(WL_ESCAN_ACTION_START);
-	p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
+	eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
+	eparams->action =  cpu_to_le16(WL_ESCAN_ACTION_START);
+	eparams->sync_id = cpu_to_le16(0x1234);
 	/* perform p2p scan on primary device */
 	ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);
 	if (!ret)
-- 
2.25.1

[PATCH v1 2/3] wifi: brcm80211: separate abort scan param prepare from actual scan

[Petr Tesarik]

From: Petr Tesarik <petr.tesarik.ext@huawei.com>

Use a dedicated helper to prepare parameters for a scan abort, rather than
special-casing a NULL argument to brcmf_escan_prep().

Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 49 ++++++++++---------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index de8a2e27f49c..3f4257fe76b3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1073,6 +1073,31 @@ static void brcmf_scan_params_v2_to_v1(struct brcmf_scan_params_v2_le *params_v2
 	       &params_v2_le->channel_list[0], params_size);
 }
 
+static void brcmf_escan_base_prep(struct brcmf_scan_params_v2_le *params_le)
+{
+	eth_broadcast_addr(params_le->bssid);
+
+	params_le->version = cpu_to_le16(BRCMF_SCAN_PARAMS_VERSION_V2);
+	params_le->bss_type = DOT11_BSSTYPE_ANY;
+	params_le->scan_type = cpu_to_le32(BRCMF_SCANTYPE_ACTIVE);
+	params_le->channel_num = 0;
+	params_le->nprobes = cpu_to_le32(-1);
+	params_le->active_time = cpu_to_le32(-1);
+	params_le->passive_time = cpu_to_le32(-1);
+	params_le->home_time = cpu_to_le32(-1);
+	memset(&params_le->ssid_le, 0, sizeof(params_le->ssid_le));
+}
+
+static void brcmf_escan_abort_prep(struct brcmf_scan_params_v2_le *params_le)
+{
+	int length = BRCMF_SCAN_PARAMS_V2_FIXED_SIZE + sizeof(u16);
+
+	brcmf_escan_base_prep(params_le);
+	params_le->channel_num = cpu_to_le32(1);
+	params_le->channel_list[0] = cpu_to_le16(-1);
+	params_le->length = cpu_to_le16(length);
+}
+
 static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
 			     struct brcmf_scan_params_v2_le *params_le,
 			     struct cfg80211_scan_request *request)
@@ -1086,29 +1111,9 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
 	int length;
 	struct brcmf_ssid_le ssid_le;
 
-	eth_broadcast_addr(params_le->bssid);
-
+	brcmf_escan_base_prep(params_le);
 	length = BRCMF_SCAN_PARAMS_V2_FIXED_SIZE;
 
-	params_le->version = cpu_to_le16(BRCMF_SCAN_PARAMS_VERSION_V2);
-	params_le->bss_type = DOT11_BSSTYPE_ANY;
-	params_le->scan_type = cpu_to_le32(BRCMF_SCANTYPE_ACTIVE);
-	params_le->channel_num = 0;
-	params_le->nprobes = cpu_to_le32(-1);
-	params_le->active_time = cpu_to_le32(-1);
-	params_le->passive_time = cpu_to_le32(-1);
-	params_le->home_time = cpu_to_le32(-1);
-	memset(&params_le->ssid_le, 0, sizeof(params_le->ssid_le));
-
-	/* Scan abort */
-	if (!request) {
-		length += sizeof(u16);
-		params_le->channel_num = cpu_to_le32(1);
-		params_le->channel_list[0] = cpu_to_le16(-1);
-		params_le->length = cpu_to_le16(length);
-		return;
-	}
-
 	n_ssids = request->n_ssids;
 	n_channels = request->n_channels;
 
@@ -1186,7 +1191,7 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 		/* Do a scan abort to stop the driver's scan engine */
 		brcmf_dbg(SCAN, "ABORT scan in firmware\n");
 
-		brcmf_escan_prep(cfg, &params_v2_le, NULL);
+		brcmf_escan_abort_prep(&params_v2_le);
 
 		/* E-Scan (or anyother type) can be aborted by SCAN */
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V2)) {
-- 
2.25.1

[PATCH v1 3/3] wifi: brcm80211: change channel_list to a flexible array

[Petr Tesarik]

From: Petr Tesarik <petr.tesarik.ext@huawei.com>

The channel list is in fact a flexible array, but it has a length of 1 to
make sure there is enough room for the special chanspec -1 when the struct
is allocated on stack to abort a scan.

Move the single array member to newly declared struct brcmf_scan_abort_le
and struct brcmf_scan_abort_v2_le and make channel_list in struct
brcmf_scan_params_le and struct brcmf_scan_params_v2_le a flexible array.

This fixes this annoying (though harmless) warning when the kernel is built
with CONFIG_FORTIFY_SOURCE=y:

------------[ cut here ]------------
memcpy: detected field-spanning write (size 76) of single field "&params_le->channel_list[0]" at drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 (size 2)
WARNING: CPU: 2 PID: 991 at drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
Modules linked in: qrtr(E) algif_hash(E) aes_neon_bs(E) aes_neon_blk(E) algif_skcipher(E) af_alg(E) bnep(E) brcmfmac_wcc(E) bcm2835_v4l2(CE) bcm2835_mmal_vchiq(CE) videobuf2_vmalloc(E) videobuf2_memops(E) videobuf2_v4l2(E) videodev(E) hci_uart(E) btsdio(E) videobuf2_common(E) btbcm(E) mc(E) snd_bcm2835(CE) bluetooth(E) snd_pcm(E) brcmfmac(E) snd_timer(E) cpufreq_dt(E) snd(E) soundcore(E) cfg80211(E) ecdh_generic(E) brcmutil(E) raspberrypi_cpufreq(E) rfkill(E) vchiq(CE) bcm2711_thermal(E) leds_gpio(E) fuse(E) efi_pstore(E) dmi_sysfs(E) ip_tables(E) x_tables(E) rpcsec_gss_krb5(E) auth_rpcgss(E) nfsv4(E) dns_resolver(E) nfs(E) lockd(E) grace(E) fscache(E) netfs(E) af_packet(E) mmc_block(E) xhci_pci(E) xhci_pci_renesas(E) xhci_hcd(E) usbcore(E) usb_common(E) clk_raspberrypi(E) gpio_raspberrypi_exp(E) bcm2835_dma(E) crct10dif_ce(E) virt_dma(E) pcie_brcmstb(E) sdhci_iproc(E) gpio_regulator(E) sdhci_pltfm(E) sdhci(E) mmc_core(E) fixed(E) nvmem_rmem(E) sunrpc(E) sg(E) dm_multipath(E) dm_mod(E) efivarfs(E)
Unloaded tainted modules: aes_ce_cipher(E):1
CPU: 2 PID: 991 Comm: wpa_supplicant Tainted: G         C  E      6.5.0-rc4-dynswiotlb+ #27 2ec0961165cc91fdbec101d9d43b3331ba4f0927
Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.04 04/01/2023
pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
lr : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
sp : ffff8000829ab590
x29: ffff8000829ab590 x28: 0000000000000000 x27: 0000000000000001
x26: ffff000105e7e0a4 x25: ffff00010a0bcb48 x24: ffff000101e03800
x23: ffff000105ec8920 x22: ffff000106332980 x21: ffff00010a0bc0c0
x20: ffff00010a0bcb90 x19: ffff00010a0bc108 x18: ffffffffffffffff
x17: 0000000000000000 x16: 0000000000000000 x15: 616f72622f737365
x14: 6c657269772f7465 x13: 616d666d6372622f x12: 31313230386d6372
x11: 00000000ffffdfff x10: ffff800081ad3328 x9 : ffff800080130694
x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
x5 : ffff0001fef75e00 x4 : 0000000000000000 x3 : 0000000000000027
x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00010aa0e000
Call trace:
 brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
 brcmf_run_escan+0x148/0x1a0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
 brcmf_do_escan+0x74/0xe0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
 brcmf_cfg80211_scan+0xcc/0x298 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
 rdev_scan+0x38/0x158 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
 cfg80211_scan+0x134/0x178 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
 nl80211_trigger_scan+0x3e8/0x768 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
 genl_family_rcv_msg_doit.isra.0+0xc0/0x130
 genl_rcv_msg+0x1e4/0x278
 netlink_rcv_skb+0x64/0x138
 genl_rcv+0x40/0x60
 netlink_unicast+0x1cc/0x2d8
 netlink_sendmsg+0x1d4/0x448
 sock_sendmsg+0x64/0xc0
 ____sys_sendmsg+0x260/0x2e0
 ___sys_sendmsg+0x88/0xf0
 __sys_sendmsg+0x70/0xd8
 __arm64_sys_sendmsg+0x2c/0x40
 invoke_syscall+0x78/0x100
 el0_svc_common.constprop.0+0x100/0x130
 do_el0_svc+0x40/0xa8
 el0_svc+0x34/0x138
 el0t_64_sync_handler+0x120/0x130
 el0t_64_sync+0x1a8/0x1b0
---[ end trace 0000000000000000 ]---

Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 22 ++++++++++---------
 .../broadcom/brcm80211/brcmfmac/fwil_types.h  | 14 ++++++++++--
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 3f4257fe76b3..337a8e915e94 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1088,8 +1088,9 @@ static void brcmf_escan_base_prep(struct brcmf_scan_params_v2_le *params_le)
 	memset(&params_le->ssid_le, 0, sizeof(params_le->ssid_le));
 }
 
-static void brcmf_escan_abort_prep(struct brcmf_scan_params_v2_le *params_le)
+static void brcmf_escan_abort_prep(struct brcmf_scan_abort_v2_le *abort_le)
 {
+	struct brcmf_scan_params_v2_le *params_le = &abort_le->params;
 	int length = BRCMF_SCAN_PARAMS_V2_FIXED_SIZE + sizeof(u16);
 
 	brcmf_escan_base_prep(params_le);
@@ -1171,7 +1172,7 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 				bool fw_abort)
 {
 	struct brcmf_pub *drvr = cfg->pub;
-	struct brcmf_scan_params_v2_le params_v2_le;
+	struct brcmf_scan_abort_v2_le abort_v2_le;
 	struct cfg80211_scan_request *scan_request;
 	u64 reqid;
 	u32 bucket;
@@ -1191,20 +1192,21 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 		/* Do a scan abort to stop the driver's scan engine */
 		brcmf_dbg(SCAN, "ABORT scan in firmware\n");
 
-		brcmf_escan_abort_prep(&params_v2_le);
+		brcmf_escan_abort_prep(&abort_v2_le);
 
-		/* E-Scan (or anyother type) can be aborted by SCAN */
+		/* E-Scan (or any other type) can be aborted by SCAN */
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V2)) {
 			err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SCAN,
-						     &params_v2_le,
-						     sizeof(params_v2_le));
+						     &abort_v2_le,
+						     sizeof(abort_v2_le));
 		} else {
-			struct brcmf_scan_params_le params_le;
+			struct brcmf_scan_abort_le abort_le;
 
-			brcmf_scan_params_v2_to_v1(&params_v2_le, &params_le);
+			brcmf_scan_params_v2_to_v1(&abort_v2_le.params,
+						   &abort_le.params);
 			err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SCAN,
-						     &params_le,
-						     sizeof(params_le));
+						     &abort_le,
+						     sizeof(abort_le));
 		}
 
 		if (err)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 792adaf880b4..10cc80b56f25 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -398,7 +398,12 @@ struct brcmf_scan_params_le {
 				 * fixed parameter portion is assumed, otherwise
 				 * ssid in the fixed portion is ignored
 				 */
-	__le16 channel_list[1];	/* list of chanspecs */
+	__le16 channel_list[];	/* list of chanspecs */
+};
+
+struct brcmf_scan_abort_le {
+	struct brcmf_scan_params_le params;
+	__le16 channel_list[1];	/* room for the special chanspec -1 */
 };
 
 struct brcmf_scan_params_v2_le {
@@ -437,7 +442,12 @@ struct brcmf_scan_params_v2_le {
 				 * fixed parameter portion is assumed, otherwise
 				 * ssid in the fixed portion is ignored
 				 */
-	__le16 channel_list[1];	/* list of chanspecs */
+	__le16 channel_list[];	/* list of chanspecs */
+};
+
+struct brcmf_scan_abort_v2_le {
+	struct brcmf_scan_params_v2_le params;
+	__le16 channel_list[1];	/* room for the special chanspec -1 */
 };
 
 struct brcmf_scan_results {
-- 
2.25.1

Re: [PATCH v1 3/3] wifi: brcm80211: change channel_list to a flexible array

[Kees Cook]

On Tue, Aug 01, 2023 at 05:36:42PM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> The channel list is in fact a flexible array, but it has a length of 1 to
> make sure there is enough room for the special chanspec -1 when the struct
> is allocated on stack to abort a scan.

There is patch series proposing a way to put flex array structures with
a compile-time-known size on the stack. Would that help for this series?
https://lore.kernel.org/netdev/20230801111923.118268-1-przemyslaw.kitszel@intel.com/

-Kees

>
-- 
Kees Cook

Re: [PATCH v1 3/3] wifi: brcm80211: change channel_list to a flexible array

[Kalle Valo]

Petr Tesarik <petrtesarik@huaweicloud.com> writes:

> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
>
> The channel list is in fact a flexible array, but it has a length of 1 to
> make sure there is enough room for the special chanspec -1 when the struct
> is allocated on stack to abort a scan.
>
> Move the single array member to newly declared struct brcmf_scan_abort_le
> and struct brcmf_scan_abort_v2_le and make channel_list in struct
> brcmf_scan_params_le and struct brcmf_scan_params_v2_le a flexible array.
>
> This fixes this annoying (though harmless) warning when the kernel is built
> with CONFIG_FORTIFY_SOURCE=y:
>
> ------------[ cut here ]------------
> memcpy: detected field-spanning write (size 76) of single field
> "&params_le->channel_list[0]" at
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 (size
> 2)
> WARNING: CPU: 2 PID: 991 at
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072
> brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
> Modules linked in: qrtr(E) algif_hash(E) aes_neon_bs(E)
> aes_neon_blk(E) algif_skcipher(E) af_alg(E) bnep(E) brcmfmac_wcc(E)
> bcm2835_v4l2(CE) bcm2835_mmal_vchiq(CE) videobuf2_vmalloc(E)
> videobuf2_memops(E) videobuf2_v4l2(E) videodev(E) hci_uart(E)
> btsdio(E) videobuf2_common(E) btbcm(E) mc(E) snd_bcm2835(CE)
> bluetooth(E) snd_pcm(E) brcmfmac(E) snd_timer(E) cpufreq_dt(E) snd(E)
> soundcore(E) cfg80211(E) ecdh_generic(E) brcmutil(E)
> raspberrypi_cpufreq(E) rfkill(E) vchiq(CE) bcm2711_thermal(E)
> leds_gpio(E) fuse(E) efi_pstore(E) dmi_sysfs(E) ip_tables(E)
> x_tables(E) rpcsec_gss_krb5(E) auth_rpcgss(E) nfsv4(E) dns_resolver(E)
> nfs(E) lockd(E) grace(E) fscache(E) netfs(E) af_packet(E) mmc_block(E)
> xhci_pci(E) xhci_pci_renesas(E) xhci_hcd(E) usbcore(E) usb_common(E)
> clk_raspberrypi(E) gpio_raspberrypi_exp(E) bcm2835_dma(E)
> crct10dif_ce(E) virt_dma(E) pcie_brcmstb(E) sdhci_iproc(E)
> gpio_regulator(E) sdhci_pltfm(E) sdhci(E) mmc_core(E) fixed(E)
> nvmem_rmem(E) sunrpc(E) sg(E) dm_multipath(E) dm_mod(E) efivarfs(E)
> Unloaded tainted modules: aes_ce_cipher(E):1
> CPU: 2 PID: 991 Comm: wpa_supplicant Tainted: G C E
> 6.5.0-rc4-dynswiotlb+ #27 2ec0961165cc91fdbec101d9d43b3331ba4f0927
> Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.04 04/01/2023
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
> lr : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
> sp : ffff8000829ab590
> x29: ffff8000829ab590 x28: 0000000000000000 x27: 0000000000000001
> x26: ffff000105e7e0a4 x25: ffff00010a0bcb48 x24: ffff000101e03800
> x23: ffff000105ec8920 x22: ffff000106332980 x21: ffff00010a0bc0c0
> x20: ffff00010a0bcb90 x19: ffff00010a0bc108 x18: ffffffffffffffff
> x17: 0000000000000000 x16: 0000000000000000 x15: 616f72622f737365
> x14: 6c657269772f7465 x13: 616d666d6372622f x12: 31313230386d6372
> x11: 00000000ffffdfff x10: ffff800081ad3328 x9 : ffff800080130694
> x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
> x5 : ffff0001fef75e00 x4 : 0000000000000000 x3 : 0000000000000027
> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00010aa0e000
> Call trace:
>  brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac
> 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>  brcmf_run_escan+0x148/0x1a0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>  brcmf_do_escan+0x74/0xe0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>  brcmf_cfg80211_scan+0xcc/0x298 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>  rdev_scan+0x38/0x158 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>  cfg80211_scan+0x134/0x178 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>  nl80211_trigger_scan+0x3e8/0x768 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>  genl_family_rcv_msg_doit.isra.0+0xc0/0x130
>  genl_rcv_msg+0x1e4/0x278
>  netlink_rcv_skb+0x64/0x138
>  genl_rcv+0x40/0x60
>  netlink_unicast+0x1cc/0x2d8
>  netlink_sendmsg+0x1d4/0x448
>  sock_sendmsg+0x64/0xc0
>  ____sys_sendmsg+0x260/0x2e0
>  ___sys_sendmsg+0x88/0xf0
>  __sys_sendmsg+0x70/0xd8
>  __arm64_sys_sendmsg+0x2c/0x40
>  invoke_syscall+0x78/0x100
>  el0_svc_common.constprop.0+0x100/0x130
>  do_el0_svc+0x40/0xa8
>  el0_svc+0x34/0x138
>  el0t_64_sync_handler+0x120/0x130
>  el0t_64_sync+0x1a8/0x1b0
> ---[ end trace 0000000000000000 ]---
>
> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>

Is this the same issue as Hans patch fixes?

https://patchwork.kernel.org/project/linux-wireless/patch/20230729140500.27892-1-hdegoede@redhat.com/

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v1 3/3] wifi: brcm80211: change channel_list to a flexible array

[Petr Tesarik]

On 8/1/2023 7:37 PM, Kalle Valo wrote:
> Petr Tesarik <petrtesarik@huaweicloud.com> writes:
> 
>> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
>>
>> The channel list is in fact a flexible array, but it has a length of 1 to
>> make sure there is enough room for the special chanspec -1 when the struct
>> is allocated on stack to abort a scan.
>>
>> Move the single array member to newly declared struct brcmf_scan_abort_le
>> and struct brcmf_scan_abort_v2_le and make channel_list in struct
>> brcmf_scan_params_le and struct brcmf_scan_params_v2_le a flexible array.
>>
>> This fixes this annoying (though harmless) warning when the kernel is built
>> with CONFIG_FORTIFY_SOURCE=y:
>>
>> ------------[ cut here ]------------
>> memcpy: detected field-spanning write (size 76) of single field
>> "&params_le->channel_list[0]" at
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072 (size
>> 2)
>> WARNING: CPU: 2 PID: 991 at
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1072
>> brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
>> Modules linked in: qrtr(E) algif_hash(E) aes_neon_bs(E)
>> aes_neon_blk(E) algif_skcipher(E) af_alg(E) bnep(E) brcmfmac_wcc(E)
>> bcm2835_v4l2(CE) bcm2835_mmal_vchiq(CE) videobuf2_vmalloc(E)
>> videobuf2_memops(E) videobuf2_v4l2(E) videodev(E) hci_uart(E)
>> btsdio(E) videobuf2_common(E) btbcm(E) mc(E) snd_bcm2835(CE)
>> bluetooth(E) snd_pcm(E) brcmfmac(E) snd_timer(E) cpufreq_dt(E) snd(E)
>> soundcore(E) cfg80211(E) ecdh_generic(E) brcmutil(E)
>> raspberrypi_cpufreq(E) rfkill(E) vchiq(CE) bcm2711_thermal(E)
>> leds_gpio(E) fuse(E) efi_pstore(E) dmi_sysfs(E) ip_tables(E)
>> x_tables(E) rpcsec_gss_krb5(E) auth_rpcgss(E) nfsv4(E) dns_resolver(E)
>> nfs(E) lockd(E) grace(E) fscache(E) netfs(E) af_packet(E) mmc_block(E)
>> xhci_pci(E) xhci_pci_renesas(E) xhci_hcd(E) usbcore(E) usb_common(E)
>> clk_raspberrypi(E) gpio_raspberrypi_exp(E) bcm2835_dma(E)
>> crct10dif_ce(E) virt_dma(E) pcie_brcmstb(E) sdhci_iproc(E)
>> gpio_regulator(E) sdhci_pltfm(E) sdhci(E) mmc_core(E) fixed(E)
>> nvmem_rmem(E) sunrpc(E) sg(E) dm_multipath(E) dm_mod(E) efivarfs(E)
>> Unloaded tainted modules: aes_ce_cipher(E):1
>> CPU: 2 PID: 991 Comm: wpa_supplicant Tainted: G C E
>> 6.5.0-rc4-dynswiotlb+ #27 2ec0961165cc91fdbec101d9d43b3331ba4f0927
>> Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.04 04/01/2023
>> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
>> lr : brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac]
>> sp : ffff8000829ab590
>> x29: ffff8000829ab590 x28: 0000000000000000 x27: 0000000000000001
>> x26: ffff000105e7e0a4 x25: ffff00010a0bcb48 x24: ffff000101e03800
>> x23: ffff000105ec8920 x22: ffff000106332980 x21: ffff00010a0bc0c0
>> x20: ffff00010a0bcb90 x19: ffff00010a0bc108 x18: ffffffffffffffff
>> x17: 0000000000000000 x16: 0000000000000000 x15: 616f72622f737365
>> x14: 6c657269772f7465 x13: 616d666d6372622f x12: 31313230386d6372
>> x11: 00000000ffffdfff x10: ffff800081ad3328 x9 : ffff800080130694
>> x8 : 000000000002ffe8 x7 : c0000000ffffdfff x6 : 00000000000affa8
>> x5 : ffff0001fef75e00 x4 : 0000000000000000 x3 : 0000000000000027
>> x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00010aa0e000
>> Call trace:
>>  brcmf_scan_params_v2_to_v1+0xd4/0x118 [brcmfmac
>> 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>>  brcmf_run_escan+0x148/0x1a0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>>  brcmf_do_escan+0x74/0xe0 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>>  brcmf_cfg80211_scan+0xcc/0x298 [brcmfmac 38c4a81a3b85b4aff1650c67f95f20bc542d60c1]
>>  rdev_scan+0x38/0x158 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>>  cfg80211_scan+0x134/0x178 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>>  nl80211_trigger_scan+0x3e8/0x768 [cfg80211 8907673111c49ec56be88af3d38994cc1cf54cb8]
>>  genl_family_rcv_msg_doit.isra.0+0xc0/0x130
>>  genl_rcv_msg+0x1e4/0x278
>>  netlink_rcv_skb+0x64/0x138
>>  genl_rcv+0x40/0x60
>>  netlink_unicast+0x1cc/0x2d8
>>  netlink_sendmsg+0x1d4/0x448
>>  sock_sendmsg+0x64/0xc0
>>  ____sys_sendmsg+0x260/0x2e0
>>  ___sys_sendmsg+0x88/0xf0
>>  __sys_sendmsg+0x70/0xd8
>>  __arm64_sys_sendmsg+0x2c/0x40
>>  invoke_syscall+0x78/0x100
>>  el0_svc_common.constprop.0+0x100/0x130
>>  do_el0_svc+0x40/0xa8
>>  el0_svc+0x34/0x138
>>  el0t_64_sync_handler+0x120/0x130
>>  el0t_64_sync+0x1a8/0x1b0
>> ---[ end trace 0000000000000000 ]---
>>
>> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> Is this the same issue as Hans patch fixes?
> 
> https://patchwork.kernel.org/project/linux-wireless/patch/20230729140500.27892-1-hdegoede@redhat.com/

Yes, I wasn't aware of this other patch (I'm not subscribed to
linux-wireless). I see, Hans approach is less intrusive.

Petr T

Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

[Petr Tesarik]

On 8/1/2023 5:36 PM, Petr Tesarik wrote:
> From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> 
> The 'S' scan method is not implemented. The type member is always written
> as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> replaced with struct brcmf_escan_params_le.

Seeing that Hans has already fixed the actual issue with the flex array,
this whole series is not needed.

But maybe this simplification has some value on its own?

Petr T

> Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> ---
>  .../broadcom/brcm80211/brcmfmac/p2p.c         | 38 ++++---------------
>  1 file changed, 8 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> index d4492d02e4ea..915f95760470 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
>  	WL_P2P_DISC_ST_SEARCH
>  };
>  
> -/**
> - * struct brcmf_p2p_scan_le - P2P specific scan request.
> - *
> - * @type: type of scan method requested (values: 'E' or 'S').
> - * @reserved: reserved (ignored).
> - * @eparams: parameters used for type 'E'.
> - * @sparams: parameters used for type 'S'.
> - */
> -struct brcmf_p2p_scan_le {
> -	u8 type;
> -	u8 reserved[3];
> -	union {
> -		struct brcmf_escan_params_le eparams;
> -		struct brcmf_scan_params_le sparams;
> -	};
> -};
> -
>  /**
>   * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
>   *
> @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
>  {
>  	struct brcmf_pub *drvr = p2p->cfg->pub;
>  	s32 ret = 0;
> -	s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> -			       eparams.params_le.channel_list);
> +	s32 memsize = offsetof(struct brcmf_escan_params_le,
> +			       params_le.channel_list);
>  	s32 nprobes;
>  	s32 active;
>  	u32 i;
>  	u8 *memblk;
>  	struct brcmf_cfg80211_vif *vif;
> -	struct brcmf_p2p_scan_le *p2p_params;
> +	struct brcmf_escan_params_le *eparams;
>  	struct brcmf_scan_params_le *sparams;
>  
>  	memsize += num_chans * sizeof(__le16);
> @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
>  		ret = -EINVAL;
>  		goto exit;
>  	}
> -	p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> -	sparams = &p2p_params->eparams.params_le;
> +	eparams = (struct brcmf_escan_params_le *)memblk;
> +	sparams = &eparams->params_le;
>  
>  	switch (search_state) {
>  	case WL_P2P_DISC_ST_SEARCH:
> @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
>  
>  	brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
>  
> -	/*
> -	 * set p2p scan parameters.
> -	 */
> -	p2p_params->type = 'E';
> -
>  	/* determine the scan engine parameters */
>  	sparams->bss_type = DOT11_BSSTYPE_ANY;
>  	sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
>  		sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
>  
>  	/* set the escan specific parameters */
> -	p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> -	p2p_params->eparams.action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> -	p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> +	eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> +	eparams->action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> +	eparams->sync_id = cpu_to_le16(0x1234);
>  	/* perform p2p scan on primary device */
>  	ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);
>  	if (!ret)

Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

[Franky Lin]

[-- Attachment #1: Type: text/plain, Size: 4426 bytes --]

On Tue, Aug 1, 2023 at 10:57 AM Petr Tesarik
<petrtesarik@huaweicloud.com> wrote:
>
> On 8/1/2023 5:36 PM, Petr Tesarik wrote:
> > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> >
> > The 'S' scan method is not implemented. The type member is always written
> > as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> > replaced with struct brcmf_escan_params_le.
>
> Seeing that Hans has already fixed the actual issue with the flex array,
> this whole series is not needed.
>
> But maybe this simplification has some value on its own?

I don't think this change would work.

> Petr T
>
> > Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > ---
> >  .../broadcom/brcm80211/brcmfmac/p2p.c         | 38 ++++---------------
> >  1 file changed, 8 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > index d4492d02e4ea..915f95760470 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
> >       WL_P2P_DISC_ST_SEARCH
> >  };
> >
> > -/**
> > - * struct brcmf_p2p_scan_le - P2P specific scan request.
> > - *
> > - * @type: type of scan method requested (values: 'E' or 'S').
> > - * @reserved: reserved (ignored).
> > - * @eparams: parameters used for type 'E'.
> > - * @sparams: parameters used for type 'S'.
> > - */
> > -struct brcmf_p2p_scan_le {
> > -     u8 type;
> > -     u8 reserved[3];
> > -     union {
> > -             struct brcmf_escan_params_le eparams;
> > -             struct brcmf_scan_params_le sparams;
> > -     };
> > -};
> > -
> >  /**
> >   * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
> >   *
> > @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> >  {
> >       struct brcmf_pub *drvr = p2p->cfg->pub;
> >       s32 ret = 0;
> > -     s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> > -                            eparams.params_le.channel_list);
> > +     s32 memsize = offsetof(struct brcmf_escan_params_le,
> > +                            params_le.channel_list);
> >       s32 nprobes;
> >       s32 active;
> >       u32 i;
> >       u8 *memblk;
> >       struct brcmf_cfg80211_vif *vif;
> > -     struct brcmf_p2p_scan_le *p2p_params;
> > +     struct brcmf_escan_params_le *eparams;
> >       struct brcmf_scan_params_le *sparams;
> >
> >       memsize += num_chans * sizeof(__le16);
> > @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> >               ret = -EINVAL;
> >               goto exit;
> >       }
> > -     p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> > -     sparams = &p2p_params->eparams.params_le;
> > +     eparams = (struct brcmf_escan_params_le *)memblk;
> > +     sparams = &eparams->params_le;
> >
> >       switch (search_state) {
> >       case WL_P2P_DISC_ST_SEARCH:
> > @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> >
> >       brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
> >
> > -     /*
> > -      * set p2p scan parameters.
> > -      */
> > -     p2p_params->type = 'E';
> > -
> >       /* determine the scan engine parameters */
> >       sparams->bss_type = DOT11_BSSTYPE_ANY;
> >       sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> > @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> >               sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
> >
> >       /* set the escan specific parameters */
> > -     p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > -     p2p_params->eparams.action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> > -     p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> > +     eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > +     eparams->action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> > +     eparams->sync_id = cpu_to_le16(0x1234);
> >       /* perform p2p scan on primary device */
> >       ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);

Firmware is expecting struct brcmf_p2p_scan_le as the parameter for
"p2p_scan" iovar.

Regards,
- Franky

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4203 bytes --]

Re: [PATCH v1 1/3] wifi: brcm80211: drop struct brcmf_p2p_scan_le

[Petr Tesařík]

On Tue, 1 Aug 2023 17:08:55 -0700
Franky Lin <franky.lin@broadcom.com> wrote:

> On Tue, Aug 1, 2023 at 10:57 AM Petr Tesarik
> <petrtesarik@huaweicloud.com> wrote:
> >
> > On 8/1/2023 5:36 PM, Petr Tesarik wrote:  
> > > From: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > >
> > > The 'S' scan method is not implemented. The type member is always written
> > > as 'E' and never read. Consequently, struct brcmf_p2p_scan_le can be
> > > replaced with struct brcmf_escan_params_le.  
> >
> > Seeing that Hans has already fixed the actual issue with the flex array,
> > this whole series is not needed.
> >
> > But maybe this simplification has some value on its own?  
> 
> I don't think this change would work.
> 
> > Petr T
> >  
> > > Signed-off-by: Petr Tesarik <petr.tesarik.ext@huawei.com>
> > > ---
> > >  .../broadcom/brcm80211/brcmfmac/p2p.c         | 38 ++++---------------
> > >  1 file changed, 8 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > index d4492d02e4ea..915f95760470 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> > > @@ -123,23 +123,6 @@ enum brcmf_p2p_disc_state {
> > >       WL_P2P_DISC_ST_SEARCH
> > >  };
> > >
> > > -/**
> > > - * struct brcmf_p2p_scan_le - P2P specific scan request.
> > > - *
> > > - * @type: type of scan method requested (values: 'E' or 'S').
> > > - * @reserved: reserved (ignored).
> > > - * @eparams: parameters used for type 'E'.
> > > - * @sparams: parameters used for type 'S'.
> > > - */
> > > -struct brcmf_p2p_scan_le {
> > > -     u8 type;
> > > -     u8 reserved[3];
> > > -     union {
> > > -             struct brcmf_escan_params_le eparams;
> > > -             struct brcmf_scan_params_le sparams;
> > > -     };
> > > -};
> > > -
> > >  /**
> > >   * struct brcmf_p2p_pub_act_frame - WiFi P2P Public Action Frame
> > >   *
> > > @@ -646,14 +629,14 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > >  {
> > >       struct brcmf_pub *drvr = p2p->cfg->pub;
> > >       s32 ret = 0;
> > > -     s32 memsize = offsetof(struct brcmf_p2p_scan_le,
> > > -                            eparams.params_le.channel_list);
> > > +     s32 memsize = offsetof(struct brcmf_escan_params_le,
> > > +                            params_le.channel_list);
> > >       s32 nprobes;
> > >       s32 active;
> > >       u32 i;
> > >       u8 *memblk;
> > >       struct brcmf_cfg80211_vif *vif;
> > > -     struct brcmf_p2p_scan_le *p2p_params;
> > > +     struct brcmf_escan_params_le *eparams;
> > >       struct brcmf_scan_params_le *sparams;
> > >
> > >       memsize += num_chans * sizeof(__le16);
> > > @@ -667,8 +650,8 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > >               ret = -EINVAL;
> > >               goto exit;
> > >       }
> > > -     p2p_params = (struct brcmf_p2p_scan_le *)memblk;
> > > -     sparams = &p2p_params->eparams.params_le;
> > > +     eparams = (struct brcmf_escan_params_le *)memblk;
> > > +     sparams = &eparams->params_le;
> > >
> > >       switch (search_state) {
> > >       case WL_P2P_DISC_ST_SEARCH:
> > > @@ -698,11 +681,6 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > >
> > >       brcmf_p2p_set_discover_state(vif->ifp, search_state, 0, 0);
> > >
> > > -     /*
> > > -      * set p2p scan parameters.
> > > -      */
> > > -     p2p_params->type = 'E';
> > > -
> > >       /* determine the scan engine parameters */
> > >       sparams->bss_type = DOT11_BSSTYPE_ANY;
> > >       sparams->scan_type = BRCMF_SCANTYPE_ACTIVE;
> > > @@ -747,9 +725,9 @@ static s32 brcmf_p2p_escan(struct brcmf_p2p_info *p2p, u32 num_chans,
> > >               sparams->channel_list[i] = cpu_to_le16(chanspecs[i]);
> > >
> > >       /* set the escan specific parameters */
> > > -     p2p_params->eparams.version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > > -     p2p_params->eparams.action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> > > -     p2p_params->eparams.sync_id = cpu_to_le16(0x1234);
> > > +     eparams->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION);
> > > +     eparams->action =  cpu_to_le16(WL_ESCAN_ACTION_START);
> > > +     eparams->sync_id = cpu_to_le16(0x1234);
> > >       /* perform p2p scan on primary device */
> > >       ret = brcmf_fil_bsscfg_data_set(vif->ifp, "p2p_scan", memblk, memsize);  
> 
> Firmware is expecting struct brcmf_p2p_scan_le as the parameter for
> "p2p_scan" iovar.

Ah, you're right, the layout is defined by the firmware protocol, and I
missed that part.

Sorry for the fuss,
Petr T