[PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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
> 

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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 {
> 

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Stanimir Varbanov]

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

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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 {
>>>
> 

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Stanimir Varbanov]

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

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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

Re: [PATCH][next] media: venus: hfi_msgs.h: Replace one-element arrays with flexible-array members

[Gustavo A. R. Silva]

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 {
>>>>>
>>>
>