| Message ID | ZgSTCmdP+omePvWg@neat |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,next] wifi: wil6210: wmi: Use __counted_by() in struct wmi_set_link_monitor_cmd and avoid -Wfamnae warning | expand |
"Gustavo A. R. Silva" <gustavoars@kernel.org> wrote: > 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 <gustavoars@kernel.org> > Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> 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
On Thu, Apr 04, 2024 at 10:12:28AM +0000, Kalle Valo wrote: > "Gustavo A. R. Silva" <gustavoars@kernel.org> wrote: > > > 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 <gustavoars@kernel.org> > > Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com> > > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > > 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
On Mon, Apr 29, 2024 at 08:25:56PM +0300, Kalle Valo wrote: > Kees Cook <keescook@chromium.org> writes: > > > On Thu, Apr 04, 2024 at 10:12:28AM +0000, Kalle Valo wrote: > > > >> "Gustavo A. R. Silva" <gustavoars@kernel.org> wrote: > >> > >> > 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 <gustavoars@kernel.org> > >> > Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com> > >> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > >> > >> 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? > > 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) 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!
On Mon, Apr 29, 2024 at 10:21:32PM +0300, Kalle Valo wrote: > Kees Cook <keescook@chromium.org> writes: > > > On Mon, Apr 29, 2024 at 08:25:56PM +0300, Kalle Valo wrote: > > > >> Kees Cook <keescook@chromium.org> writes: > >> > >> > On Thu, Apr 04, 2024 at 10:12:28AM +0000, Kalle Valo wrote: > >> > > >> >> "Gustavo A. R. Silva" <gustavoars@kernel.org> wrote: > >> >> > >> >> > 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 <gustavoars@kernel.org> > >> >> > Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com> > >> >> > Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com> > >> >> > >> >> 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? > >> > >> 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) > > > > 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.
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c index 6fdb77d4c59e..8ff69dc72fb9 100644 --- a/drivers/net/wireless/ath/wil6210/wmi.c +++ b/drivers/net/wireless/ath/wil6210/wmi.c @@ -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); diff --git a/drivers/net/wireless/ath/wil6210/wmi.h b/drivers/net/wireless/ath/wil6210/wmi.h index b47606d9068c..38f64524019e 100644 --- a/drivers/net/wireless/ath/wil6210/wmi.h +++ b/drivers/net/wireless/ath/wil6210/wmi.h @@ -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 */
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 <gustavoars@kernel.org> --- 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(-)