linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ath11k: Replace one-element array with flexible-array member
@ 2021-09-04 11:49 Len Baker
  2021-09-12 19:31 ` Gustavo A. R. Silva
  0 siblings, 1 reply; 4+ messages in thread
From: Len Baker @ 2021-09-04 11:49 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller, Jakub Kicinski
  Cc: Len Baker, Gustavo A. R. Silva, Kees Cook, ath11k,
	linux-wireless, netdev, linux-hardening, linux-kernel

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].

Also, refactor the code a bit to make use of the struct_size() helper in
kzalloc().

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#zero-length-and-one-element-arrays

Signed-off-by: Len Baker <len.baker@gmx.com>
---
 drivers/net/wireless/ath/ath11k/reg.c | 7 ++-----
 drivers/net/wireless/ath/ath11k/wmi.h | 2 +-
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index e1a1df169034..c83d265185f1 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -97,7 +97,6 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
 	struct channel_param *ch;
 	enum nl80211_band band;
 	int num_channels = 0;
-	int params_len;
 	int i, ret;

 	bands = hw->wiphy->bands;
@@ -117,10 +116,8 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
 	if (WARN_ON(!num_channels))
 		return -EINVAL;

-	params_len = sizeof(struct scan_chan_list_params) +
-			num_channels * sizeof(struct channel_param);
-	params = kzalloc(params_len, GFP_KERNEL);
-
+	params = kzalloc(struct_size(params, ch_param, num_channels),
+			 GFP_KERNEL);
 	if (!params)
 		return -ENOMEM;

diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
index d35c47e0b19d..d9c83726f65d 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.h
+++ b/drivers/net/wireless/ath/ath11k/wmi.h
@@ -3608,7 +3608,7 @@ struct wmi_stop_scan_cmd {
 struct scan_chan_list_params {
 	u32 pdev_id;
 	u16 nallchans;
-	struct channel_param ch_param[1];
+	struct channel_param ch_param[];
 };

 struct wmi_scan_chan_list_cmd {
--
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ath11k: Replace one-element array with flexible-array member
  2021-09-04 11:49 [PATCH] ath11k: Replace one-element array with flexible-array member Len Baker
@ 2021-09-12 19:31 ` Gustavo A. R. Silva
  2021-09-12 19:42   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2021-09-12 19:31 UTC (permalink / raw)
  To: Len Baker
  Cc: Kalle Valo, David S. Miller, Jakub Kicinski, Gustavo A. R. Silva,
	Kees Cook, ath11k, linux-wireless, netdev, linux-hardening,
	linux-kernel


There is already a patch for this:

https://lore.kernel.org/lkml/20210823172159.GA25800@embeddedor/

which I will now add to my -next tree.

Thanks
--
Gustavo

On Sat, Sep 04, 2021 at 01:49:37PM +0200, Len Baker 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].
> 
> Also, refactor the code a bit to make use of the struct_size() helper in
> kzalloc().
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Signed-off-by: Len Baker <len.baker@gmx.com>
> ---
>  drivers/net/wireless/ath/ath11k/reg.c | 7 ++-----
>  drivers/net/wireless/ath/ath11k/wmi.h | 2 +-
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
> index e1a1df169034..c83d265185f1 100644
> --- a/drivers/net/wireless/ath/ath11k/reg.c
> +++ b/drivers/net/wireless/ath/ath11k/reg.c
> @@ -97,7 +97,6 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
>  	struct channel_param *ch;
>  	enum nl80211_band band;
>  	int num_channels = 0;
> -	int params_len;
>  	int i, ret;
> 
>  	bands = hw->wiphy->bands;
> @@ -117,10 +116,8 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
>  	if (WARN_ON(!num_channels))
>  		return -EINVAL;
> 
> -	params_len = sizeof(struct scan_chan_list_params) +
> -			num_channels * sizeof(struct channel_param);
> -	params = kzalloc(params_len, GFP_KERNEL);
> -
> +	params = kzalloc(struct_size(params, ch_param, num_channels),
> +			 GFP_KERNEL);
>  	if (!params)
>  		return -ENOMEM;
> 
> diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
> index d35c47e0b19d..d9c83726f65d 100644
> --- a/drivers/net/wireless/ath/ath11k/wmi.h
> +++ b/drivers/net/wireless/ath/ath11k/wmi.h
> @@ -3608,7 +3608,7 @@ struct wmi_stop_scan_cmd {
>  struct scan_chan_list_params {
>  	u32 pdev_id;
>  	u16 nallchans;
> -	struct channel_param ch_param[1];
> +	struct channel_param ch_param[];
>  };
> 
>  struct wmi_scan_chan_list_cmd {
> --
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ath11k: Replace one-element array with flexible-array member
  2021-09-12 19:31 ` Gustavo A. R. Silva
@ 2021-09-12 19:42   ` Gustavo A. R. Silva
  2021-09-14  5:53     ` Kalle Valo
  0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2021-09-12 19:42 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Len Baker
  Cc: Kalle Valo, David S. Miller, Jakub Kicinski, Kees Cook, ath11k,
	linux-wireless, netdev, linux-hardening, linux-kernel



On 9/12/21 14:31, Gustavo A. R. Silva wrote:
> 
> There is already a patch for this:
> 
> https://lore.kernel.org/lkml/20210823172159.GA25800@embeddedor/
> 
> which I will now add to my -next tree.

Well, in this case I think it's much better if Kalle can take it. :)

--
Gustavo
> On Sat, Sep 04, 2021 at 01:49:37PM +0200, Len Baker 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].
>>
>> Also, refactor the code a bit to make use of the struct_size() helper in
>> kzalloc().
>>
>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>> [2] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#zero-length-and-one-element-arrays
>>
>> Signed-off-by: Len Baker <len.baker@gmx.com>
>> ---
>>  drivers/net/wireless/ath/ath11k/reg.c | 7 ++-----
>>  drivers/net/wireless/ath/ath11k/wmi.h | 2 +-
>>  2 files changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
>> index e1a1df169034..c83d265185f1 100644
>> --- a/drivers/net/wireless/ath/ath11k/reg.c
>> +++ b/drivers/net/wireless/ath/ath11k/reg.c
>> @@ -97,7 +97,6 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
>>  	struct channel_param *ch;
>>  	enum nl80211_band band;
>>  	int num_channels = 0;
>> -	int params_len;
>>  	int i, ret;
>>
>>  	bands = hw->wiphy->bands;
>> @@ -117,10 +116,8 @@ int ath11k_reg_update_chan_list(struct ath11k *ar)
>>  	if (WARN_ON(!num_channels))
>>  		return -EINVAL;
>>
>> -	params_len = sizeof(struct scan_chan_list_params) +
>> -			num_channels * sizeof(struct channel_param);
>> -	params = kzalloc(params_len, GFP_KERNEL);
>> -
>> +	params = kzalloc(struct_size(params, ch_param, num_channels),
>> +			 GFP_KERNEL);
>>  	if (!params)
>>  		return -ENOMEM;
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
>> index d35c47e0b19d..d9c83726f65d 100644
>> --- a/drivers/net/wireless/ath/ath11k/wmi.h
>> +++ b/drivers/net/wireless/ath/ath11k/wmi.h
>> @@ -3608,7 +3608,7 @@ struct wmi_stop_scan_cmd {
>>  struct scan_chan_list_params {
>>  	u32 pdev_id;
>>  	u16 nallchans;
>> -	struct channel_param ch_param[1];
>> +	struct channel_param ch_param[];
>>  };
>>
>>  struct wmi_scan_chan_list_cmd {
>> --
>> 2.25.1
>>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ath11k: Replace one-element array with flexible-array member
  2021-09-12 19:42   ` Gustavo A. R. Silva
@ 2021-09-14  5:53     ` Kalle Valo
  0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-09-14  5:53 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Gustavo A. R. Silva, Len Baker, David S. Miller, Jakub Kicinski,
	Kees Cook, ath11k, linux-wireless, netdev, linux-hardening,
	linux-kernel

"Gustavo A. R. Silva" <gustavo@embeddedor.com> writes:

> On 9/12/21 14:31, Gustavo A. R. Silva wrote:
>> 
>> There is already a patch for this:
>> 
>> https://lore.kernel.org/lkml/20210823172159.GA25800@embeddedor/
>> 
>> which I will now add to my -next tree.
>
> Well, in this case I think it's much better if Kalle can take it. :)

Please don't take any ath11k patches. I have a ton of ath11k patches in
my queue and if you start taking some of them, it will cause conflicts
between trees.

Only take wireless-driver patches which I have acked, please.

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

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-14  5:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 11:49 [PATCH] ath11k: Replace one-element array with flexible-array member Len Baker
2021-09-12 19:31 ` Gustavo A. R. Silva
2021-09-12 19:42   ` Gustavo A. R. Silva
2021-09-14  5:53     ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).