wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning

PATCH In review Gustavo A. R. Silva 4 participants 9 messages Mar 27, 2024 – Apr 30, 2024 Wilocity WIL6210 wireless driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2024-03-27 21:43 UTC #0 lore raw

PATCH v2 1/1 wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).

Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.

So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.

So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Link https://github.com/KSPP/linux/issues/202

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
- Use __struct_size() to get the compile-time size of the flex-struct instance.

v1:
- Link: https://lore.kernel.org/linux-hardening/ZgRsn72WkHzfCUsa@neat/

 drivers/net/wireless/ath/wil6210/wmi.c | 19 +++++++------------
 drivers/net/wireless/ath/wil6210/wmi.h |  2 +-
 2 files changed, 8 insertions(+), 13 deletions(-)

drivers/net/wireless/ath/wil6210/wmi.c +7 ~~-12~~

index 6fdb77d4c59e..8ff69dc72fb9 100644
@@ -4014,28 +4014,23 @@ int wmi_set_cqm_rssi_config(struct wil6210_priv *wil,
 	struct net_device *ndev = wil->main_ndev;
 	struct wil6210_vif *vif = ndev_to_vif(ndev);
 	int rc;
-	struct {
-		struct wmi_set_link_monitor_cmd cmd;
-		s8 rssi_thold;
-	} __packed cmd = {
-		.cmd = {
-			.rssi_hyst = rssi_hyst,
-			.rssi_thresholds_list_size = 1,
-		},
-		.rssi_thold = rssi_thold,
-	};
 	struct {
 		struct wmi_cmd_hdr hdr;
 		struct wmi_set_link_monitor_event evt;
 	} __packed reply = {
 		.evt = {.status = WMI_FW_STATUS_FAILURE},
 	};
+	DEFINE_FLEX(struct wmi_set_link_monitor_cmd, cmd,
+		    rssi_thresholds_list, rssi_thresholds_list_size, 1);
+
+	cmd->rssi_hyst = rssi_hyst;
+	cmd->rssi_thresholds_list[0] = rssi_thold;
 
 	if (rssi_thold > S8_MAX || rssi_thold < S8_MIN || rssi_hyst > U8_MAX)
 		return -EINVAL;
 
-	rc = wmi_call(wil, WMI_SET_LINK_MONITOR_CMDID, vif->mid, &cmd,
-		      sizeof(cmd), WMI_SET_LINK_MONITOR_EVENTID,
+	rc = wmi_call(wil, WMI_SET_LINK_MONITOR_CMDID, vif->mid, cmd,
+		      __struct_size(cmd), WMI_SET_LINK_MONITOR_EVENTID,
 		      &reply, sizeof(reply), WIL_WMI_CALL_GENERAL_TO_MS);
 	if (rc) {
 		wil_err(wil, "WMI_SET_LINK_MONITOR_CMDID failed, rc %d\n", rc);

drivers/net/wireless/ath/wil6210/wmi.h +1 -1

index b47606d9068c..38f64524019e 100644
@@ -3320,7 +3320,7 @@ struct wmi_set_link_monitor_cmd {
 	u8 rssi_hyst;
 	u8 reserved[12];
 	u8 rssi_thresholds_list_size;
-	s8 rssi_thresholds_list[];
+	s8 rssi_thresholds_list[] __counted_by(rssi_thresholds_list_size);
 } __packed;
 
 /* wmi_link_monitor_event_type */

Signature · 1 line

2.34.1

Jeff Johnson

Reviewed-by on v2 1/1

2024-03-27 21:57 UTC #1 lore raw

Quoted 14 lines · Gustavo A. R. Silva

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.

Quoted 5 lines

So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>

Reviewed-by Jeff Johnson <[email protected]>

Kalle Valo

2024-04-04 10:12 UTC #2 lore raw

In reply to Gustavo A. R. Silva

Quoted 14 lines · Gustavo A. R. Silva

Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.

Quoted 7 lines

So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning

Signature · 3 lines

https://patchwork.kernel.org/project/linux-wireless/patch/ZgSTCmdP+omePvWg@neat/

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

Kees Cook

2024-04-29 17:10 UTC #3 lore raw

Quoted 19 lines · Kalle Valo

Quoted 17 lines · Gustavo A. R. Silva
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.
So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]

Quoted 8 lines

Quoted 4 lines
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning

Hi,

I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?

Thanks!

-Kees

Signature · 1 line

Kees Cook

Kalle Valo

2024-04-29 17:25 UTC #4 lore raw

Quoted 30 lines · Kees Cook

Quoted 28 lines · Kalle Valo
Quoted 19 lines · Gustavo A. R. Silva
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.
So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Quoted 4 lines
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.

Quoted 8 lines

Quoted 2 lines
cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning
Hi,
I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?

Yeah, wireless-next was pulled last week so most likely we will create ath-next pull request this week.

BTW we are planning to move ath.git to a new location, rename branches etc. I think we'll see if we can also setup it so that it can be pulled to linux-next, so that you don't need to ask this every time ;)

(Just joking of course, there a lot of benefits from having the tree in linux-next)

Signature · 3 lines

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

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

Kees Cook

2024-04-29 18:09 UTC #5 lore raw

Quoted 44 lines · Kalle Valo

Quoted 30 lines · Kees Cook
Quoted 28 lines · Kalle Valo
Quoted 19 lines · Gustavo A. R. Silva
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.
So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Quoted 4 lines
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
Quoted 8 lines
Quoted 2 lines
cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning
Hi,
I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?
Yeah, wireless-next was pulled last week so most likely we will create ath-next pull request this week.

Quoted 6 lines

BTW we are planning to move ath.git to a new location, rename branches etc. I think we'll see if we can also setup it so that it can be pulled to linux-next, so that you don't need to ask this every time ;)
(Just joking of course, there a lot of benefits from having the tree in linux-next)

Ah-ha! Thanks. Yeah, sorry if I keep asking about that. It's different from other trees, so it doesn't stick in my head. :) I should keep better notes!

Signature · 1 line

Kees Cook

Kalle Valo

2024-04-29 19:21 UTC #6 lore raw

Quoted 50 lines · Kees Cook

Quoted 48 lines · Kalle Valo
Quoted 30 lines · Kees Cook
Quoted 28 lines · Kalle Valo
Quoted 19 lines · Gustavo A. R. Silva
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.
So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Quoted 4 lines
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
Quoted 8 lines
Quoted 2 lines
cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning
Hi,
I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?
Yeah, wireless-next was pulled last week so most likely we will create ath-next pull request this week.
BTW we are planning to move ath.git to a new location, rename branches etc. I think we'll see if we can also setup it so that it can be pulled to linux-next, so that you don't need to ask this every time ;)

Quoted 6 lines

Quoted 2 lines
(Just joking of course, there a lot of benefits from having the tree in linux-next)
Ah-ha! Thanks. Yeah, sorry if I keep asking about that. It's different from other trees, so it doesn't stick in my head. :) I should keep better notes!

BTW I think all vendor specific wireless driver trees are not pulled to
linux-next: iwlwifi, mt76, rtw (Realtek) and ath. So with all of these it will take a while before the commit is in linux-next.

Signature · 3 lines

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

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

Kees Cook

2024-04-29 19:52 UTC #7 lore raw

Quoted 55 lines · Kalle Valo

Quoted 53 lines · Kees Cook
Quoted 50 lines · Kalle Valo
Quoted 30 lines · Kees Cook
Quoted 28 lines · Kalle Valo
Quoted 19 lines · Gustavo A. R. Silva
Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions).
Also, -Wflex-array-member-not-at-end is coming in GCC-14, and we are getting ready to enable it globally.
So, use the DEFINE_FLEX() helper for an on-stack definition of a flexible structure where the size of the flexible-array member is known at compile-time, and refactor the rest of the code, accordingly.
So, with these changes, fix the following warning:
drivers/net/wireless/ath/wil6210/wmi.c:4018:49: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
Quoted 4 lines
Link https://github.com/KSPP/linux/issues/202
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Jeff Johnson <[email protected]>
Signed-off-by Kalle Valo <[email protected]>
Patch applied to ath-next branch of ath.git, thanks.
Quoted 8 lines
Quoted 2 lines
cbb0697e0ded wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning
Hi,
I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?
Yeah, wireless-next was pulled last week so most likely we will create ath-next pull request this week.
BTW we are planning to move ath.git to a new location, rename branches etc. I think we'll see if we can also setup it so that it can be pulled to linux-next, so that you don't need to ask this every time ;)
(Just joking of course, there a lot of benefits from having the tree in linux-next)

Quoted 7 lines

Quoted 3 lines
Ah-ha! Thanks. Yeah, sorry if I keep asking about that. It's different from other trees, so it doesn't stick in my head. :) I should keep better notes!
BTW I think all vendor specific wireless driver trees are not pulled to
linux-next: iwlwifi, mt76, rtw (Realtek) and ath. So with all of these it will take a while before the commit is in linux-next.

How long is "a while"? And if the latency can be reduced for these, it'd be nice since it would allow for longer bake-time in -next.

Signature · 1 line

Kees Cook

Kalle Valo

2024-04-30 06:39 UTC #8 lore raw

Quoted 18 lines · Kees Cook

Quoted 18 lines
Quoted 17 lines
Quoted 7 lines
Quoted 3 lines
I was just walking through our patch tracker and noticed that I don't see this patch include in -next yet (as of next-20240429). Is there a flush of the ath-next queue planned soon? Or did I miss some change?
Yeah, wireless-next was pulled last week so most likely we will create ath-next pull request this week.
Quoted 6 lines
BTW we are planning to move ath.git to a new location, rename branches etc. I think we'll see if we can also setup it so that it can be pulled to linux-next, so that you don't need to ask this every time ;)
(Just joking of course, there a lot of benefits from having the tree in linux-next)
Ah-ha! Thanks. Yeah, sorry if I keep asking about that. It's different from other trees, so it doesn't stick in my head. :) I should keep better notes!

Quoted 5 lines

Quoted 3 lines
BTW I think all vendor specific wireless driver trees are not pulled to
linux-next: iwlwifi, mt76, rtw (Realtek) and ath. So with all of these it will take a while before the commit is in linux-next.
How long is "a while"?

The cadence can be anything from 1-4 times per release (~8 weeks). Depends on the maintainer, how busy we are etc.

Quoted 2 lines

And if the latency can be reduced for these, it'd be nice since it would allow for longer bake-time in -next.

Sure but our time is limited, as always :) There's extra overhead with linux-next, like the rule that no updates during the merge window, so I can understand why some maintainers have not included their tree to linux-next builds.

Signature · 3 lines

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

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