All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][next] i40e: Replace one-element array with flexible-array member
@ 2021-05-25 23:00 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-25 23:00 UTC (permalink / raw)
  To: Faisal Latif, Shiraz Saleem, Doug Ledford, Jason Gunthorpe,
	Jesse Brandeburg, Tony Nguyen, David S. Miller, Jakub Kicinski
  Cc: linux-rdma, linux-kernel, intel-wired-lan, netdev,
	Gustavo A. R. Silva, linux-hardening

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

Refactor the code according to the use of a flexible-array member in struct
i40e_qvlist_info instead of one-element array, and use the struct_size()
helper.

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.10/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/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
 include/linux/net/intel/i40e_client.h         | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index b496f30ce066..364f69cd620f 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1423,7 +1423,7 @@ static enum i40iw_status_code i40iw_save_msix_info(struct i40iw_device *iwdev,
 	struct i40e_qv_info *iw_qvinfo;
 	u32 ceq_idx;
 	u32 i;
-	u32 size;
+	size_t size;
 
 	if (!ldev->msix_count) {
 		i40iw_pr_err("No MSI-X vectors\n");
@@ -1433,8 +1433,7 @@ static enum i40iw_status_code i40iw_save_msix_info(struct i40iw_device *iwdev,
 	iwdev->msix_count = ldev->msix_count;
 
 	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
-	size += sizeof(struct i40e_qvlist_info);
-	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
+	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
 	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
 
 	if (!iwdev->iw_msixtbl)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 32f3facbed1a..63eab14a26df 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
 	u32 v_idx, i, reg_idx, reg;
 
 	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
-				    qvlist_info->num_vectors - 1), GFP_KERNEL);
+				    qvlist_info->num_vectors), GFP_KERNEL);
 	if (!ldev->qvlist_info)
 		return -ENOMEM;
 	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors;
diff --git a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
index f41387a8969f..fd7bc860a241 100644
--- a/include/linux/net/intel/i40e_client.h
+++ b/include/linux/net/intel/i40e_client.h
@@ -48,7 +48,7 @@ struct i40e_qv_info {
 
 struct i40e_qvlist_info {
 	u32 num_vectors;
-	struct i40e_qv_info qv_info[1];
+	struct i40e_qv_info qv_info[];
 };
 
 
-- 
2.27.0


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

* [Intel-wired-lan] [PATCH][next] i40e: Replace one-element array with flexible-array member
@ 2021-05-25 23:00 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-25 23:00 UTC (permalink / raw)
  To: intel-wired-lan

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

Refactor the code according to the use of a flexible-array member in struct
i40e_qvlist_info instead of one-element array, and use the struct_size()
helper.

[1] https://en.wikipedia.org/wiki/Flexible_array_member
[2] https://www.kernel.org/doc/html/v5.10/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/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
 drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
 include/linux/net/intel/i40e_client.h         | 2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c
index b496f30ce066..364f69cd620f 100644
--- a/drivers/infiniband/hw/i40iw/i40iw_main.c
+++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
@@ -1423,7 +1423,7 @@ static enum i40iw_status_code i40iw_save_msix_info(struct i40iw_device *iwdev,
 	struct i40e_qv_info *iw_qvinfo;
 	u32 ceq_idx;
 	u32 i;
-	u32 size;
+	size_t size;
 
 	if (!ldev->msix_count) {
 		i40iw_pr_err("No MSI-X vectors\n");
@@ -1433,8 +1433,7 @@ static enum i40iw_status_code i40iw_save_msix_info(struct i40iw_device *iwdev,
 	iwdev->msix_count = ldev->msix_count;
 
 	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
-	size += sizeof(struct i40e_qvlist_info);
-	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
+	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
 	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
 
 	if (!iwdev->iw_msixtbl)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c b/drivers/net/ethernet/intel/i40e/i40e_client.c
index 32f3facbed1a..63eab14a26df 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_client.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
@@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
 	u32 v_idx, i, reg_idx, reg;
 
 	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
-				    qvlist_info->num_vectors - 1), GFP_KERNEL);
+				    qvlist_info->num_vectors), GFP_KERNEL);
 	if (!ldev->qvlist_info)
 		return -ENOMEM;
 	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors;
diff --git a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
index f41387a8969f..fd7bc860a241 100644
--- a/include/linux/net/intel/i40e_client.h
+++ b/include/linux/net/intel/i40e_client.h
@@ -48,7 +48,7 @@ struct i40e_qv_info {
 
 struct i40e_qvlist_info {
 	u32 num_vectors;
-	struct i40e_qv_info qv_info[1];
+	struct i40e_qv_info qv_info[];
 };
 
 
-- 
2.27.0


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

* RE: [PATCH][next] i40e: Replace one-element array with flexible-array member
  2021-05-25 23:00 ` [Intel-wired-lan] " Gustavo A. R. Silva
@ 2021-05-26 21:29   ` Saleem, Shiraz
  -1 siblings, 0 replies; 7+ messages in thread
From: Saleem, Shiraz @ 2021-05-26 21:29 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Latif, Faisal, Doug Ledford,
	Jason Gunthorpe, Brandeburg, Jesse, Nguyen, Anthony L,
	David S. Miller, Jakub Kicinski
  Cc: linux-rdma, linux-kernel, intel-wired-lan, netdev, linux-hardening

> Subject: [PATCH][next] i40e: Replace one-element array with flexible-array member
> 
> 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].
> 
> Refactor the code according to the use of a flexible-array member in struct
> i40e_qvlist_info instead of one-element array, and use the struct_size() helper.
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.10/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>

This looks ok to me.
Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>

It seems we should add this to the new irdma driver submission as well which replaces i40iw.
I will fold it into v7 of the rdma portion of the series
https://lore.kernel.org/linux-rdma/20210520200326.GX1096940@ziepe.ca/

Additionally we will add this patch when we resend this PR on iwl-next.
https://lore.kernel.org/linux-rdma/62555c6de641e10cb4169653731389a51d086345.camel@intel.com/


> ---
>  drivers/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
>  drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
>  include/linux/net/intel/i40e_client.h         | 2 +-
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c
> b/drivers/infiniband/hw/i40iw/i40iw_main.c
> index b496f30ce066..364f69cd620f 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
> @@ -1423,7 +1423,7 @@ static enum i40iw_status_code
> i40iw_save_msix_info(struct i40iw_device *iwdev,
>  	struct i40e_qv_info *iw_qvinfo;
>  	u32 ceq_idx;
>  	u32 i;
> -	u32 size;
> +	size_t size;
> 
>  	if (!ldev->msix_count) {
>  		i40iw_pr_err("No MSI-X vectors\n");
> @@ -1433,8 +1433,7 @@ static enum i40iw_status_code
> i40iw_save_msix_info(struct i40iw_device *iwdev,
>  	iwdev->msix_count = ldev->msix_count;
> 
>  	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
> -	size += sizeof(struct i40e_qvlist_info);
> -	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
> +	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
>  	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
> 
>  	if (!iwdev->iw_msixtbl)
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
> b/drivers/net/ethernet/intel/i40e/i40e_client.c
> index 32f3facbed1a..63eab14a26df 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
> @@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
>  	u32 v_idx, i, reg_idx, reg;
> 
>  	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
> -				    qvlist_info->num_vectors - 1), GFP_KERNEL);
> +				    qvlist_info->num_vectors), GFP_KERNEL);
>  	if (!ldev->qvlist_info)
>  		return -ENOMEM;
>  	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors; diff --git
> a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
> index f41387a8969f..fd7bc860a241 100644
> --- a/include/linux/net/intel/i40e_client.h
> +++ b/include/linux/net/intel/i40e_client.h
> @@ -48,7 +48,7 @@ struct i40e_qv_info {
> 
>  struct i40e_qvlist_info {
>  	u32 num_vectors;
> -	struct i40e_qv_info qv_info[1];
> +	struct i40e_qv_info qv_info[];
>  };
> 
> 
> --
> 2.27.0


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

* [Intel-wired-lan] [PATCH][next] i40e: Replace one-element array with flexible-array member
@ 2021-05-26 21:29   ` Saleem, Shiraz
  0 siblings, 0 replies; 7+ messages in thread
From: Saleem, Shiraz @ 2021-05-26 21:29 UTC (permalink / raw)
  To: intel-wired-lan

> Subject: [PATCH][next] i40e: Replace one-element array with flexible-array member
> 
> 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].
> 
> Refactor the code according to the use of a flexible-array member in struct
> i40e_qvlist_info instead of one-element array, and use the struct_size() helper.
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.10/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>

This looks ok to me.
Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>

It seems we should add this to the new irdma driver submission as well which replaces i40iw.
I will fold it into v7 of the rdma portion of the series
https://lore.kernel.org/linux-rdma/20210520200326.GX1096940 at ziepe.ca/

Additionally we will add this patch when we resend this PR on iwl-next.
https://lore.kernel.org/linux-rdma/62555c6de641e10cb4169653731389a51d086345.camel at intel.com/


> ---
>  drivers/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
>  drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
>  include/linux/net/intel/i40e_client.h         | 2 +-
>  3 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c
> b/drivers/infiniband/hw/i40iw/i40iw_main.c
> index b496f30ce066..364f69cd620f 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
> @@ -1423,7 +1423,7 @@ static enum i40iw_status_code
> i40iw_save_msix_info(struct i40iw_device *iwdev,
>  	struct i40e_qv_info *iw_qvinfo;
>  	u32 ceq_idx;
>  	u32 i;
> -	u32 size;
> +	size_t size;
> 
>  	if (!ldev->msix_count) {
>  		i40iw_pr_err("No MSI-X vectors\n");
> @@ -1433,8 +1433,7 @@ static enum i40iw_status_code
> i40iw_save_msix_info(struct i40iw_device *iwdev,
>  	iwdev->msix_count = ldev->msix_count;
> 
>  	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
> -	size += sizeof(struct i40e_qvlist_info);
> -	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
> +	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
>  	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
> 
>  	if (!iwdev->iw_msixtbl)
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
> b/drivers/net/ethernet/intel/i40e/i40e_client.c
> index 32f3facbed1a..63eab14a26df 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
> @@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
>  	u32 v_idx, i, reg_idx, reg;
> 
>  	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
> -				    qvlist_info->num_vectors - 1), GFP_KERNEL);
> +				    qvlist_info->num_vectors), GFP_KERNEL);
>  	if (!ldev->qvlist_info)
>  		return -ENOMEM;
>  	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors; diff --git
> a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
> index f41387a8969f..fd7bc860a241 100644
> --- a/include/linux/net/intel/i40e_client.h
> +++ b/include/linux/net/intel/i40e_client.h
> @@ -48,7 +48,7 @@ struct i40e_qv_info {
> 
>  struct i40e_qvlist_info {
>  	u32 num_vectors;
> -	struct i40e_qv_info qv_info[1];
> +	struct i40e_qv_info qv_info[];
>  };
> 
> 
> --
> 2.27.0


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

* [Intel-wired-lan] [PATCH][next] i40e: Replace one-element array with flexible-array member
  2021-05-26 21:29   ` [Intel-wired-lan] " Saleem, Shiraz
  (?)
@ 2021-05-26 21:34   ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-26 21:34 UTC (permalink / raw)
  To: intel-wired-lan



On 5/26/21 16:29, Saleem, Shiraz wrote:
>> Subject: [PATCH][next] i40e: Replace one-element array with flexible-array member
>>
>> 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].
>>
>> Refactor the code according to the use of a flexible-array member in struct
>> i40e_qvlist_info instead of one-element array, and use the struct_size() helper.
>>
>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>> [2] https://www.kernel.org/doc/html/v5.10/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>
> 
> This looks ok to me.
> Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>
> 
> It seems we should add this to the new irdma driver submission as well which replaces i40iw.
> I will fold it into v7 of the rdma portion of the series
> https://lore.kernel.org/linux-rdma/20210520200326.GX1096940 at ziepe.ca/

OK. Just please, when you fold it, add my Signed-off-by tag like this:

[flexible array transformation]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> 
> Additionally we will add this patch when we resend this PR on iwl-next.
> https://lore.kernel.org/linux-rdma/62555c6de641e10cb4169653731389a51d086345.camel at intel.com/
> 
> 
>> ---
>>  drivers/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
>>  drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
>>  include/linux/net/intel/i40e_client.h         | 2 +-
>>  3 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> index b496f30ce066..364f69cd620f 100644
>> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> @@ -1423,7 +1423,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	struct i40e_qv_info *iw_qvinfo;
>>  	u32 ceq_idx;
>>  	u32 i;
>> -	u32 size;
>> +	size_t size;
>>
>>  	if (!ldev->msix_count) {
>>  		i40iw_pr_err("No MSI-X vectors\n");
>> @@ -1433,8 +1433,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	iwdev->msix_count = ldev->msix_count;
>>
>>  	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
>> -	size += sizeof(struct i40e_qvlist_info);
>> -	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
>> +	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
>>  	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
>>
>>  	if (!iwdev->iw_msixtbl)
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> index 32f3facbed1a..63eab14a26df 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> @@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
>>  	u32 v_idx, i, reg_idx, reg;
>>
>>  	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
>> -				    qvlist_info->num_vectors - 1), GFP_KERNEL);
>> +				    qvlist_info->num_vectors), GFP_KERNEL);
>>  	if (!ldev->qvlist_info)
>>  		return -ENOMEM;
>>  	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors; diff --git
>> a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
>> index f41387a8969f..fd7bc860a241 100644
>> --- a/include/linux/net/intel/i40e_client.h
>> +++ b/include/linux/net/intel/i40e_client.h
>> @@ -48,7 +48,7 @@ struct i40e_qv_info {
>>
>>  struct i40e_qvlist_info {
>>  	u32 num_vectors;
>> -	struct i40e_qv_info qv_info[1];
>> +	struct i40e_qv_info qv_info[];
>>  };
>>
>>
>> --
>> 2.27.0
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 

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

* Re: [Intel-wired-lan] [PATCH][next] i40e: Replace one-element array with flexible-array member
  2021-05-26 21:29   ` [Intel-wired-lan] " Saleem, Shiraz
@ 2021-05-26 21:35     ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-26 21:35 UTC (permalink / raw)
  To: Saleem, Shiraz, Gustavo A. R. Silva, Latif, Faisal, Doug Ledford,
	Jason Gunthorpe, Brandeburg, Jesse, Nguyen, Anthony L,
	David S. Miller, Jakub Kicinski
  Cc: linux-rdma, netdev, intel-wired-lan, linux-kernel, linux-hardening



On 5/26/21 16:29, Saleem, Shiraz wrote:
>> Subject: [PATCH][next] i40e: Replace one-element array with flexible-array member
>>
>> 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].
>>
>> Refactor the code according to the use of a flexible-array member in struct
>> i40e_qvlist_info instead of one-element array, and use the struct_size() helper.
>>
>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>> [2] https://www.kernel.org/doc/html/v5.10/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>
> 
> This looks ok to me.
> Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>
> 
> It seems we should add this to the new irdma driver submission as well which replaces i40iw.
> I will fold it into v7 of the rdma portion of the series
> https://lore.kernel.org/linux-rdma/20210520200326.GX1096940@ziepe.ca/

OK. Just please, when you fold it, add my Signed-off-by tag like this:

[flexible array transformation]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> Additionally we will add this patch when we resend this PR on iwl-next.
> https://lore.kernel.org/linux-rdma/62555c6de641e10cb4169653731389a51d086345.camel@intel.com/
> 
> 
>> ---
>>  drivers/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
>>  drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
>>  include/linux/net/intel/i40e_client.h         | 2 +-
>>  3 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> index b496f30ce066..364f69cd620f 100644
>> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> @@ -1423,7 +1423,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	struct i40e_qv_info *iw_qvinfo;
>>  	u32 ceq_idx;
>>  	u32 i;
>> -	u32 size;
>> +	size_t size;
>>
>>  	if (!ldev->msix_count) {
>>  		i40iw_pr_err("No MSI-X vectors\n");
>> @@ -1433,8 +1433,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	iwdev->msix_count = ldev->msix_count;
>>
>>  	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
>> -	size += sizeof(struct i40e_qvlist_info);
>> -	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
>> +	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
>>  	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
>>
>>  	if (!iwdev->iw_msixtbl)
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> index 32f3facbed1a..63eab14a26df 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> @@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
>>  	u32 v_idx, i, reg_idx, reg;
>>
>>  	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
>> -				    qvlist_info->num_vectors - 1), GFP_KERNEL);
>> +				    qvlist_info->num_vectors), GFP_KERNEL);
>>  	if (!ldev->qvlist_info)
>>  		return -ENOMEM;
>>  	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors; diff --git
>> a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
>> index f41387a8969f..fd7bc860a241 100644
>> --- a/include/linux/net/intel/i40e_client.h
>> +++ b/include/linux/net/intel/i40e_client.h
>> @@ -48,7 +48,7 @@ struct i40e_qv_info {
>>
>>  struct i40e_qvlist_info {
>>  	u32 num_vectors;
>> -	struct i40e_qv_info qv_info[1];
>> +	struct i40e_qv_info qv_info[];
>>  };
>>
>>
>> --
>> 2.27.0
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 

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

* [Intel-wired-lan] [PATCH][next] i40e: Replace one-element array with flexible-array member
@ 2021-05-26 21:35     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 7+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-26 21:35 UTC (permalink / raw)
  To: intel-wired-lan



On 5/26/21 16:29, Saleem, Shiraz wrote:
>> Subject: [PATCH][next] i40e: Replace one-element array with flexible-array member
>>
>> 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].
>>
>> Refactor the code according to the use of a flexible-array member in struct
>> i40e_qvlist_info instead of one-element array, and use the struct_size() helper.
>>
>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>> [2] https://www.kernel.org/doc/html/v5.10/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>
> 
> This looks ok to me.
> Acked-by: Shiraz Saleem <shiraz.saleem@intel.com>
> 
> It seems we should add this to the new irdma driver submission as well which replaces i40iw.
> I will fold it into v7 of the rdma portion of the series
> https://lore.kernel.org/linux-rdma/20210520200326.GX1096940 at ziepe.ca/

OK. Just please, when you fold it, add my Signed-off-by tag like this:

[flexible array transformation]
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> Additionally we will add this patch when we resend this PR on iwl-next.
> https://lore.kernel.org/linux-rdma/62555c6de641e10cb4169653731389a51d086345.camel at intel.com/
> 
> 
>> ---
>>  drivers/infiniband/hw/i40iw/i40iw_main.c      | 5 ++---
>>  drivers/net/ethernet/intel/i40e/i40e_client.c | 2 +-
>>  include/linux/net/intel/i40e_client.h         | 2 +-
>>  3 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> index b496f30ce066..364f69cd620f 100644
>> --- a/drivers/infiniband/hw/i40iw/i40iw_main.c
>> +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c
>> @@ -1423,7 +1423,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	struct i40e_qv_info *iw_qvinfo;
>>  	u32 ceq_idx;
>>  	u32 i;
>> -	u32 size;
>> +	size_t size;
>>
>>  	if (!ldev->msix_count) {
>>  		i40iw_pr_err("No MSI-X vectors\n");
>> @@ -1433,8 +1433,7 @@ static enum i40iw_status_code
>> i40iw_save_msix_info(struct i40iw_device *iwdev,
>>  	iwdev->msix_count = ldev->msix_count;
>>
>>  	size = sizeof(struct i40iw_msix_vector) * iwdev->msix_count;
>> -	size += sizeof(struct i40e_qvlist_info);
>> -	size +=  sizeof(struct i40e_qv_info) * iwdev->msix_count - 1;
>> +	size += struct_size(iw_qvlist, qv_info, iwdev->msix_count);
>>  	iwdev->iw_msixtbl = kzalloc(size, GFP_KERNEL);
>>
>>  	if (!iwdev->iw_msixtbl)
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> index 32f3facbed1a..63eab14a26df 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_client.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_client.c
>> @@ -579,7 +579,7 @@ static int i40e_client_setup_qvlist(struct i40e_info *ldev,
>>  	u32 v_idx, i, reg_idx, reg;
>>
>>  	ldev->qvlist_info = kzalloc(struct_size(ldev->qvlist_info, qv_info,
>> -				    qvlist_info->num_vectors - 1), GFP_KERNEL);
>> +				    qvlist_info->num_vectors), GFP_KERNEL);
>>  	if (!ldev->qvlist_info)
>>  		return -ENOMEM;
>>  	ldev->qvlist_info->num_vectors = qvlist_info->num_vectors; diff --git
>> a/include/linux/net/intel/i40e_client.h b/include/linux/net/intel/i40e_client.h
>> index f41387a8969f..fd7bc860a241 100644
>> --- a/include/linux/net/intel/i40e_client.h
>> +++ b/include/linux/net/intel/i40e_client.h
>> @@ -48,7 +48,7 @@ struct i40e_qv_info {
>>
>>  struct i40e_qvlist_info {
>>  	u32 num_vectors;
>> -	struct i40e_qv_info qv_info[1];
>> +	struct i40e_qv_info qv_info[];
>>  };
>>
>>
>> --
>> 2.27.0
> 
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan at osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
> 

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

end of thread, other threads:[~2021-05-26 21:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25 23:00 [PATCH][next] i40e: Replace one-element array with flexible-array member Gustavo A. R. Silva
2021-05-25 23:00 ` [Intel-wired-lan] " Gustavo A. R. Silva
2021-05-26 21:29 ` Saleem, Shiraz
2021-05-26 21:29   ` [Intel-wired-lan] " Saleem, Shiraz
2021-05-26 21:34   ` Gustavo A. R. Silva
2021-05-26 21:35   ` Gustavo A. R. Silva
2021-05-26 21:35     ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.