linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
@ 2021-02-11  0:10 Gustavo A. R. Silva
  2021-03-04  1:43 ` Gustavo A. R. Silva
  2021-05-11 15:46 ` Gustavo A. R. Silva
  0 siblings, 2 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-02-11  0:10 UTC (permalink / raw)
  To: Stanimir Varbanov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, 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].

Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
hfi_msg_session_property_info_pkt instead of one-element arrays.

Also, this helps with the ongoing efforts to enable -Warray-bounds by
fixing the following warnings:

  CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
  246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
      |                          ~~~~~~~~~^~~
drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
  342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
      |                                                     ~~~~~~~~~^~~

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

Link: https://github.com/KSPP/linux/issues/79
Link: https://github.com/KSPP/linux/issues/109
Build-tested-by: <lkp@intel.com>
Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
index 526d9f5b487b..e2d2ccfbdd24 100644
--- a/drivers/media/platform/qcom/venus/hfi_msgs.h
+++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
@@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
 struct hfi_msg_sys_property_info_pkt {
 	struct hfi_pkt_hdr hdr;
 	u32 num_properties;
-	u32 data[1];
+	u32 data[];
 };
 
 struct hfi_msg_session_load_resources_done_pkt {
@@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
 struct hfi_msg_session_property_info_pkt {
 	struct hfi_session_hdr_pkt shdr;
 	u32 num_properties;
-	u32 data[1];
+	u32 data[];
 };
 
 struct hfi_msg_session_release_resources_done_pkt {
-- 
2.27.0


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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-02-11  0:10 [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
@ 2021-03-04  1:43 ` Gustavo A. R. Silva
  2021-05-11 15:46 ` Gustavo A. R. Silva
  1 sibling, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-03-04  1:43 UTC (permalink / raw)
  To: Stanimir Varbanov, Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi all,

Friendly ping: who can take this, please?

Thanks
--
Gustavo

On Wed, Feb 10, 2021 at 06:10:44PM -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].
> 
> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
> hfi_msg_session_property_info_pkt instead of one-element arrays.
> 
> Also, this helps with the ongoing efforts to enable -Warray-bounds by
> fixing the following warnings:
> 
>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>       |                          ~~~~~~~~~^~~
> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>       |                                                     ~~~~~~~~~^~~
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/109
> Build-tested-by: <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
> index 526d9f5b487b..e2d2ccfbdd24 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>  struct hfi_msg_sys_property_info_pkt {
>  	struct hfi_pkt_hdr hdr;
>  	u32 num_properties;
> -	u32 data[1];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_load_resources_done_pkt {
> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>  struct hfi_msg_session_property_info_pkt {
>  	struct hfi_session_hdr_pkt shdr;
>  	u32 num_properties;
> -	u32 data[1];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_release_resources_done_pkt {
> -- 
> 2.27.0
> 

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-02-11  0:10 [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
  2021-03-04  1:43 ` Gustavo A. R. Silva
@ 2021-05-11 15:46 ` Gustavo A. R. Silva
  2021-05-12 11:39   ` Stanimir Varbanov
  1 sibling, 1 reply; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-11 15:46 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Stanimir Varbanov, Andy Gross,
	Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi all,

Friendly ping:

We are about to be able to globally enable -Warray-bounds and, these are one of
the last out-of-bounds warnings in linux-next.

Could someone take this, please?

Thanks
--
Gustavo

On 2/10/21 18:10, 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].
> 
> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
> hfi_msg_session_property_info_pkt instead of one-element arrays.
> 
> Also, this helps with the ongoing efforts to enable -Warray-bounds by
> fixing the following warnings:
> 
>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>       |                          ~~~~~~~~~^~~
> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>       |                                                     ~~~~~~~~~^~~
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/109
> Build-tested-by: <lkp@intel.com>
> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
> index 526d9f5b487b..e2d2ccfbdd24 100644
> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>  struct hfi_msg_sys_property_info_pkt {
>  	struct hfi_pkt_hdr hdr;
>  	u32 num_properties;
> -	u32 data[1];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_load_resources_done_pkt {
> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>  struct hfi_msg_session_property_info_pkt {
>  	struct hfi_session_hdr_pkt shdr;
>  	u32 num_properties;
> -	u32 data[1];
> +	u32 data[];
>  };
>  
>  struct hfi_msg_session_release_resources_done_pkt {
> 

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-05-11 15:46 ` Gustavo A. R. Silva
@ 2021-05-12 11:39   ` Stanimir Varbanov
  2021-05-12 16:37     ` Gustavo A. R. Silva
  0 siblings, 1 reply; 8+ messages in thread
From: Stanimir Varbanov @ 2021-05-12 11:39 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Gustavo A. R. Silva, Stanimir Varbanov,
	Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi,

On 5/11/21 6:46 PM, Gustavo A. R. Silva wrote:
> Hi all,
> 
> Friendly ping:
> 
> We are about to be able to globally enable -Warray-bounds and, these are one of
> the last out-of-bounds warnings in linux-next.
> 
> Could someone take this, please?

This one introduces regressions, so I cannot take it. It needs some more
work.

> 
> Thanks
> --
> Gustavo
> 
> On 2/10/21 18:10, 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].
>>
>> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
>> hfi_msg_session_property_info_pkt instead of one-element arrays.
>>
>> Also, this helps with the ongoing efforts to enable -Warray-bounds by
>> fixing the following warnings:
>>
>>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
>> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>>       |                          ~~~~~~~~~^~~
>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
>> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>>       |                                                     ~~~~~~~~~^~~
>>
>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
>>
>> Link: https://github.com/KSPP/linux/issues/79
>> Link: https://github.com/KSPP/linux/issues/109
>> Build-tested-by: <lkp@intel.com>
>> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
>> index 526d9f5b487b..e2d2ccfbdd24 100644
>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
>> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>>  struct hfi_msg_sys_property_info_pkt {
>>  	struct hfi_pkt_hdr hdr;
>>  	u32 num_properties;
>> -	u32 data[1];
>> +	u32 data[];
>>  };
>>  
>>  struct hfi_msg_session_load_resources_done_pkt {
>> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>>  struct hfi_msg_session_property_info_pkt {
>>  	struct hfi_session_hdr_pkt shdr;
>>  	u32 num_properties;
>> -	u32 data[1];
>> +	u32 data[];
>>  };
>>  
>>  struct hfi_msg_session_release_resources_done_pkt {
>>

-- 
regards,
Stan

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-05-12 11:39   ` Stanimir Varbanov
@ 2021-05-12 16:37     ` Gustavo A. R. Silva
  2021-05-17 11:01       ` Stanimir Varbanov
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-12 16:37 UTC (permalink / raw)
  To: Stanimir Varbanov, Gustavo A. R. Silva, Andy Gross,
	Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening



On 5/12/21 06:39, Stanimir Varbanov wrote:
> Hi,
> 
> On 5/11/21 6:46 PM, Gustavo A. R. Silva wrote:
>> Hi all,
>>
>> Friendly ping:
>>
>> We are about to be able to globally enable -Warray-bounds and, these are one of
>> the last out-of-bounds warnings in linux-next.
>>
>> Could someone take this, please?
> 
> This one introduces regressions, so I cannot take it. It needs some more
> work.

Please, share with me the errors or warnings you see with this. So, I can
have an idea of what is going on. Unfortunately, I don't have access to the
test suite or hardware to test this.

Thanks!
--
Gustavo



>>
>> Thanks
>> --
>> Gustavo
>>
>> On 2/10/21 18:10, 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].
>>>
>>> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
>>> hfi_msg_session_property_info_pkt instead of one-element arrays.
>>>
>>> Also, this helps with the ongoing efforts to enable -Warray-bounds by
>>> fixing the following warnings:
>>>
>>>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
>>> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>>>       |                          ~~~~~~~~~^~~
>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
>>> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>>>       |                                                     ~~~~~~~~~^~~
>>>
>>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>>> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
>>>
>>> Link: https://github.com/KSPP/linux/issues/79
>>> Link: https://github.com/KSPP/linux/issues/109
>>> Build-tested-by: <lkp@intel.com>
>>> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>> ---
>>>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>> index 526d9f5b487b..e2d2ccfbdd24 100644
>>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
>>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>>>  struct hfi_msg_sys_property_info_pkt {
>>>  	struct hfi_pkt_hdr hdr;
>>>  	u32 num_properties;
>>> -	u32 data[1];
>>> +	u32 data[];
>>>  };
>>>  
>>>  struct hfi_msg_session_load_resources_done_pkt {
>>> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>>>  struct hfi_msg_session_property_info_pkt {
>>>  	struct hfi_session_hdr_pkt shdr;
>>>  	u32 num_properties;
>>> -	u32 data[1];
>>> +	u32 data[];
>>>  };
>>>  
>>>  struct hfi_msg_session_release_resources_done_pkt {
>>>
> 

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-05-12 16:37     ` Gustavo A. R. Silva
@ 2021-05-17 11:01       ` Stanimir Varbanov
  2021-05-19 16:15         ` Gustavo A. R. Silva
  2021-06-04  0:47         ` Gustavo A. R. Silva
  0 siblings, 2 replies; 8+ messages in thread
From: Stanimir Varbanov @ 2021-05-17 11:01 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Stanimir Varbanov, Gustavo A. R. Silva,
	Andy Gross, Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi,

On 5/12/21 7:37 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 5/12/21 06:39, Stanimir Varbanov wrote:
>> Hi,
>>
>> On 5/11/21 6:46 PM, Gustavo A. R. Silva wrote:
>>> Hi all,
>>>
>>> Friendly ping:
>>>
>>> We are about to be able to globally enable -Warray-bounds and, these are one of
>>> the last out-of-bounds warnings in linux-next.
>>>
>>> Could someone take this, please?
>>
>> This one introduces regressions, so I cannot take it. It needs some more
>> work.
> 
> Please, share with me the errors or warnings you see with this. So, I can
> have an idea of what is going on. Unfortunately, I don't have access to the
> test suite or hardware to test this.

I guess it needs more debugging, but the simple answer is that the
driver refuse to start streaming with this patch.

> 
> Thanks!
> --
> Gustavo
> 
> 
> 
>>>
>>> Thanks
>>> --
>>> Gustavo
>>>
>>> On 2/10/21 18:10, 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].
>>>>
>>>> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
>>>> hfi_msg_session_property_info_pkt instead of one-element arrays.
>>>>
>>>> Also, this helps with the ongoing efforts to enable -Warray-bounds by
>>>> fixing the following warnings:
>>>>
>>>>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
>>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
>>>> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>>>>       |                          ~~~~~~~~~^~~
>>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
>>>> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>>>>       |                                                     ~~~~~~~~~^~~
>>>>
>>>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>>>> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
>>>>
>>>> Link: https://github.com/KSPP/linux/issues/79
>>>> Link: https://github.com/KSPP/linux/issues/109
>>>> Build-tested-by: <lkp@intel.com>
>>>> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>>> ---
>>>>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>> index 526d9f5b487b..e2d2ccfbdd24 100644
>>>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>>>>  struct hfi_msg_sys_property_info_pkt {
>>>>  	struct hfi_pkt_hdr hdr;
>>>>  	u32 num_properties;
>>>> -	u32 data[1];
>>>> +	u32 data[];
>>>>  };
>>>>  
>>>>  struct hfi_msg_session_load_resources_done_pkt {
>>>> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>>>>  struct hfi_msg_session_property_info_pkt {
>>>>  	struct hfi_session_hdr_pkt shdr;
>>>>  	u32 num_properties;
>>>> -	u32 data[1];
>>>> +	u32 data[];
>>>>  };
>>>>  
>>>>  struct hfi_msg_session_release_resources_done_pkt {
>>>>
>>

-- 
regards,
Stan

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-05-17 11:01       ` Stanimir Varbanov
@ 2021-05-19 16:15         ` Gustavo A. R. Silva
  2021-06-04  0:47         ` Gustavo A. R. Silva
  1 sibling, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-19 16:15 UTC (permalink / raw)
  To: Stanimir Varbanov, Gustavo A. R. Silva, Andy Gross,
	Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi Stanimir,

On 5/17/21 06:01, Stanimir Varbanov wrote:

>>> This one introduces regressions, so I cannot take it. It needs some more
>>> work.
>>
>> Please, share with me the errors or warnings you see with this. So, I can
>> have an idea of what is going on. Unfortunately, I don't have access to the
>> test suite or hardware to test this.
> 
> I guess it needs more debugging, but the simple answer is that the
> driver refuse to start streaming with this patch.

I see. Please, share with me the details about your test setup. Are you using
qemu or actual hardware for this?

Please, also help me by sending me your .config file and dmesg output, as well.
So, I can try to debug this.

Thanks!
--
Gustavo

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

* Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members
  2021-05-17 11:01       ` Stanimir Varbanov
  2021-05-19 16:15         ` Gustavo A. R. Silva
@ 2021-06-04  0:47         ` Gustavo A. R. Silva
  1 sibling, 0 replies; 8+ messages in thread
From: Gustavo A. R. Silva @ 2021-06-04  0:47 UTC (permalink / raw)
  To: Stanimir Varbanov, Gustavo A. R. Silva, Andy Gross,
	Bjorn Andersson, Mauro Carvalho Chehab
  Cc: linux-media, linux-arm-msm, linux-kernel, linux-hardening

Hi Stanimir,

On 5/17/21 06:01, Stanimir Varbanov wrote:
> Hi,
> 
> On 5/12/21 7:37 PM, Gustavo A. R. Silva wrote:
>>
>>
>> On 5/12/21 06:39, Stanimir Varbanov wrote:
>>> Hi,
>>>
>>> On 5/11/21 6:46 PM, Gustavo A. R. Silva wrote:
>>>> Hi all,
>>>>
>>>> Friendly ping:
>>>>
>>>> We are about to be able to globally enable -Warray-bounds and, these are one of
>>>> the last out-of-bounds warnings in linux-next.
>>>>
>>>> Could someone take this, please?
>>>
>>> This one introduces regressions, so I cannot take it. It needs some more
>>> work.
>>
>> Please, share with me the errors or warnings you see with this. So, I can
>> have an idea of what is going on. Unfortunately, I don't have access to the
>> test suite or hardware to test this.
> 
> I guess it needs more debugging, but the simple answer is that the
> driver refuse to start streaming with this patch.

This patch should fix the problems you are seeing:

https://lore.kernel.org/linux-hardening/20210604004338.GA140710@embeddedor/

Could you please help me to test it?

Thanks!
--
Gustavo

> 
>>
>> Thanks!
>> --
>> Gustavo
>>
>>
>>
>>>>
>>>> Thanks
>>>> --
>>>> Gustavo
>>>>
>>>> On 2/10/21 18:10, 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].
>>>>>
>>>>> Use flexible-array members in struct hfi_msg_sys_property_info_pkt and
>>>>> hfi_msg_session_property_info_pkt instead of one-element arrays.
>>>>>
>>>>> Also, this helps with the ongoing efforts to enable -Warray-bounds by
>>>>> fixing the following warnings:
>>>>>
>>>>>   CC [M]  drivers/media/platform/qcom/venus/hfi_msgs.o
>>>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_sys_property_info’:
>>>>> drivers/media/platform/qcom/venus/hfi_msgs.c:246:35: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>>>   246 |  if (req_bytes < 128 || !pkt->data[1] || pkt->num_properties > 1)
>>>>>       |                          ~~~~~~~~~^~~
>>>>> drivers/media/platform/qcom/venus/hfi_msgs.c: In function ‘hfi_session_prop_info’:
>>>>> drivers/media/platform/qcom/venus/hfi_msgs.c:342:62: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>>>>   342 |  if (!req_bytes || req_bytes % sizeof(*buf_req) || !pkt->data[1])
>>>>>       |                                                     ~~~~~~~~~^~~
>>>>>
>>>>> [1] https://en.wikipedia.org/wiki/Flexible_array_member
>>>>> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
>>>>>
>>>>> Link: https://github.com/KSPP/linux/issues/79
>>>>> Link: https://github.com/KSPP/linux/issues/109
>>>>> Build-tested-by: <lkp@intel.com>
>>>>> Link: https://lore.kernel.org/lkml/6023dd80.MmTeFf8SzwX0iK7%2F%25lkp@intel.com/
>>>>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>>>>> ---
>>>>>  drivers/media/platform/qcom/venus/hfi_msgs.h | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>>> index 526d9f5b487b..e2d2ccfbdd24 100644
>>>>> --- a/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>>> +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h
>>>>> @@ -113,7 +113,7 @@ struct hfi_msg_sys_ping_ack_pkt {
>>>>>  struct hfi_msg_sys_property_info_pkt {
>>>>>  	struct hfi_pkt_hdr hdr;
>>>>>  	u32 num_properties;
>>>>> -	u32 data[1];
>>>>> +	u32 data[];
>>>>>  };
>>>>>  
>>>>>  struct hfi_msg_session_load_resources_done_pkt {
>>>>> @@ -233,7 +233,7 @@ struct hfi_msg_session_parse_sequence_header_done_pkt {
>>>>>  struct hfi_msg_session_property_info_pkt {
>>>>>  	struct hfi_session_hdr_pkt shdr;
>>>>>  	u32 num_properties;
>>>>> -	u32 data[1];
>>>>> +	u32 data[];
>>>>>  };
>>>>>  
>>>>>  struct hfi_msg_session_release_resources_done_pkt {
>>>>>
>>>
> 

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

end of thread, other threads:[~2021-06-04  1:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11  0:10 [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members Gustavo A. R. Silva
2021-03-04  1:43 ` Gustavo A. R. Silva
2021-05-11 15:46 ` Gustavo A. R. Silva
2021-05-12 11:39   ` Stanimir Varbanov
2021-05-12 16:37     ` Gustavo A. R. Silva
2021-05-17 11:01       ` Stanimir Varbanov
2021-05-19 16:15         ` Gustavo A. R. Silva
2021-06-04  0:47         ` Gustavo A. R. Silva

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox