| Message ID | 20220204233015.GA454979@embeddedor (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] staging: wlan-ng: Replace one-element arrays with flexible-array members | expand |
On Fri, Feb 04, 2022 at 05:30:15PM -0600, Gustavo A. R. Silva wrote: > There is a regular need in the kernel to provide a way to declare having > a dynamically sized set of trailing elements in a structure. Kernel code > should always use “flexible array members”[1] for these cases. The older > style of one-element or zero-length arrays should no longer be used[2]. > > This helps with the ongoing efforts to globally enable -Warray-bounds > and get us closer to being able to tighten the FORTIFY_SOURCE routines > on memcpy(). > > This issue was found with the help of Coccinelle and audited and fixed, > manually. How are you auditing this manually? I have to re-audit it manually myself as a reviewer and I'm find it tricky. > /*-- IBSS Parameter Set ---------------------------*/ > @@ -288,7 +288,7 @@ struct wlan_ie_ibss_parms { > struct wlan_ie_challenge { > u8 eid; > u8 len; > - u8 challenge[1]; > + u8 challenge[]; > } __packed; This is dead code. It would have been better to just delete that. I have a lot of experience at reviewing "Delete Dead Code" patches. regards, dan carpenter
diff --git a/drivers/staging/wlan-ng/p80211mgmt.h b/drivers/staging/wlan-ng/p80211mgmt.h index 1ef30d3f3159..d6fe52de2c8f 100644 --- a/drivers/staging/wlan-ng/p80211mgmt.h +++ b/drivers/staging/wlan-ng/p80211mgmt.h @@ -229,14 +229,14 @@ struct wlan_ie { struct wlan_ie_ssid { u8 eid; u8 len; - u8 ssid[1]; /* may be zero, ptrs may overlap */ + u8 ssid[]; /* may be zero, ptrs may overlap */ } __packed; /*-- Supported Rates -----------------------------*/ struct wlan_ie_supp_rates { u8 eid; u8 len; - u8 rates[1]; /* had better be at LEAST one! */ + u8 rates[]; /* had better be at LEAST one! */ } __packed; /*-- FH Parameter Set ----------------------------*/ @@ -274,7 +274,7 @@ struct wlan_ie_tim { u8 dtim_cnt; u8 dtim_period; u8 bitmap_ctl; - u8 virt_bm[1]; + u8 virt_bm[]; } __packed; /*-- IBSS Parameter Set ---------------------------*/ @@ -288,7 +288,7 @@ struct wlan_ie_ibss_parms { struct wlan_ie_challenge { u8 eid; u8 len; - u8 challenge[1]; + u8 challenge[]; } __packed; /*-------------------------------------------------*/ diff --git a/drivers/staging/wlan-ng/p80211types.h b/drivers/staging/wlan-ng/p80211types.h index 6486612a8f31..b2ffd09881b0 100644 --- a/drivers/staging/wlan-ng/p80211types.h +++ b/drivers/staging/wlan-ng/p80211types.h @@ -234,7 +234,7 @@ struct p80211pstr32 { /* MAC address array */ struct p80211macarray { u32 cnt; - u8 data[1][MAXLEN_PSTR6]; + u8 data[][MAXLEN_PSTR6]; } __packed; /* prototype template */
There is a regular need in the kernel to provide a way to declare having a dynamically sized set of trailing elements in a structure. Kernel code should always use “flexible array members”[1] for these cases. The older style of one-element or zero-length arrays should no longer be used[2]. This helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). This issue was found with the help of Coccinelle and audited and fixed, manually. [1] https://en.wikipedia.org/wiki/Flexible_array_member [2] https://www.kernel.org/doc/html/v5.16/process/deprecated.html#zero-length-and-one-element-arrays Link: https://github.com/KSPP/linux/issues/79 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/staging/wlan-ng/p80211mgmt.h | 8 ++++---- drivers/staging/wlan-ng/p80211types.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)