On 3/27/2024 10:43 AM, Gustavo A. R. Silva 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). > > Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx> > --- > Changes in v2: > - Annotate one more struct. > - Update Subject line. > > v1: > - Link: https://lore.kernel.org/linux-hardening/ZgODZOB4fOBvKl7R@neat/ > > drivers/net/wireless/ath/wil6210/wmi.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/wil6210/wmi.h b/drivers/net/wireless/ath/wil6210/wmi.h > index 71bf2ae27a98..38f64524019e 100644 > --- a/drivers/net/wireless/ath/wil6210/wmi.h > +++ b/drivers/net/wireless/ath/wil6210/wmi.h > @@ -474,7 +474,7 @@ struct wmi_start_scan_cmd { > struct { > u8 channel; > u8 reserved; > - } channel_list[]; > + } channel_list[] __counted_by(num_channels); > } __packed; does the compiler handle the actual logic where it is modifying num_channels concurrently with writing into the array? i.e. this will be writing into channel_list[0] when num_channels is 0: cmd.cmd.channel_list[cmd.cmd.num_channels++].channel = ch - 1; if that will cause a bounds check failure then suggest you change the logic so that it updates num_channels before writing into channel_list > > #define WMI_MAX_PNO_SSID_NUM (16) > @@ -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; this looks ok to me, although I think there is another issue associated with this, namely the way the code populates the rssi_thresholds_list is by defining a separate anonymous struct: 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, }; I would expect gcc and clang to both complain about that s8 rssi_thold comes after a flexible array (even though its purpose is to be the value of rssi_thresholds_list[0]) /jeff > > /* wmi_link_monitor_event_type */
