linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND][next] media: siano: Fix out-of-bounds warnings in smscore_load_firmware_family2()
@ 2021-05-11 17:45 Gustavo A. R. Silva
  2021-06-01 22:12 ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2021-05-11 17:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Gustavo A. R. Silva, linux-hardening,
	Kees Cook

Rename struct sms_msg_data4 to sms_msg_data5 and increase the size of
its msg_data array from 4 to 5 elements. Notice that at some point
the 5th element of msg_data is being accessed in function
smscore_load_firmware_family2():

1006                 trigger_msg->msg_data[4] = 4; /* Task ID */

Also, there is no need for the object _trigger_msg_ of type struct
sms_msg_data *, when _msg_ can be used, directly. Notice that msg_data
in struct sms_msg_data is a one-element array, which causes multiple
out-of-bounds warnings when accessing beyond its first element
in function smscore_load_firmware_family2():

 992                 struct sms_msg_data *trigger_msg =                                                  
 993                         (struct sms_msg_data *) msg;                                                
 994                                                                                                     
 995                 pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");                               
 996                 SMS_INIT_MSG(&msg->x_msg_header,                                                    
 997                                 MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,                                     
 998                                 sizeof(struct sms_msg_hdr) +                                        
 999                                 sizeof(u32) * 5);                                                   
1000                                                                                                     
1001                 trigger_msg->msg_data[0] = firmware->start_address;                                 
1002                                         /* Entry point */                                           
1003                 trigger_msg->msg_data[1] = 6; /* Priority */                                        
1004                 trigger_msg->msg_data[2] = 0x200; /* Stack size */                                  
1005                 trigger_msg->msg_data[3] = 0; /* Parameter */                                       
1006                 trigger_msg->msg_data[4] = 4; /* Task ID */ 

even when enough dynamic memory is allocated for _msg_:

 929         /* PAGE_SIZE buffer shall be enough and dma aligned */
 930         msg = kmalloc(PAGE_SIZE, GFP_KERNEL | coredev->gfp_buf_flags);

but as _msg_ is casted to (struct sms_msg_data *):

 992                 struct sms_msg_data *trigger_msg =
 993                         (struct sms_msg_data *) msg;

the out-of-bounds warnings are actually valid and should be addressed.

Fix this by declaring object _msg_ of type struct sms_msg_data5 *,
which contains a 5-elements array, instead of just 4. And use
_msg_ directly, instead of creating object trigger_msg.

This helps with the ongoing efforts to enable -Warray-bounds by fixing
the following warnings:

  CC [M]  drivers/media/common/siano/smscoreapi.o
drivers/media/common/siano/smscoreapi.c: In function ‘smscore_load_firmware_family2’:
drivers/media/common/siano/smscoreapi.c:1003:24: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
 1003 |   trigger_msg->msg_data[1] = 6; /* Priority */
      |   ~~~~~~~~~~~~~~~~~~~~~^~~
In file included from drivers/media/common/siano/smscoreapi.c:12:
drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
  619 |  u32 msg_data[1];
      |      ^~~~~~~~
drivers/media/common/siano/smscoreapi.c:1004:24: warning: array subscript 2 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
 1004 |   trigger_msg->msg_data[2] = 0x200; /* Stack size */
      |   ~~~~~~~~~~~~~~~~~~~~~^~~
In file included from drivers/media/common/siano/smscoreapi.c:12:
drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
  619 |  u32 msg_data[1];
      |      ^~~~~~~~
drivers/media/common/siano/smscoreapi.c:1005:24: warning: array subscript 3 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
 1005 |   trigger_msg->msg_data[3] = 0; /* Parameter */
      |   ~~~~~~~~~~~~~~~~~~~~~^~~
In file included from drivers/media/common/siano/smscoreapi.c:12:
drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
  619 |  u32 msg_data[1];
      |      ^~~~~~~~
drivers/media/common/siano/smscoreapi.c:1006:24: warning: array subscript 4 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
 1006 |   trigger_msg->msg_data[4] = 4; /* Task ID */
      |   ~~~~~~~~~~~~~~~~~~~~~^~~
In file included from drivers/media/common/siano/smscoreapi.c:12:
drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
  619 |  u32 msg_data[1];
      |      ^~~~~~~~

Fixes: 018b0c6f8acb ("[media] siano: make load firmware logic to work with newer firmwares")
Co-developed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
Hi,

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

Thanks

 drivers/media/common/siano/smscoreapi.c | 22 +++++++++-------------
 drivers/media/common/siano/smscoreapi.h |  4 ++--
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/media/common/siano/smscoreapi.c b/drivers/media/common/siano/smscoreapi.c
index 410cc3ac6f94..bceaf91faa15 100644
--- a/drivers/media/common/siano/smscoreapi.c
+++ b/drivers/media/common/siano/smscoreapi.c
@@ -908,7 +908,7 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
 					 void *buffer, size_t size)
 {
 	struct sms_firmware *firmware = (struct sms_firmware *) buffer;
-	struct sms_msg_data4 *msg;
+	struct sms_msg_data5 *msg;
 	u32 mem_address,  calc_checksum = 0;
 	u32 i, *ptr;
 	u8 *payload = firmware->payload;
@@ -989,24 +989,20 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
 		goto exit_fw_download;
 
 	if (coredev->mode == DEVICE_MODE_NONE) {
-		struct sms_msg_data *trigger_msg =
-			(struct sms_msg_data *) msg;
-
 		pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");
 		SMS_INIT_MSG(&msg->x_msg_header,
 				MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,
-				sizeof(struct sms_msg_hdr) +
-				sizeof(u32) * 5);
+				sizeof(*msg));
 
-		trigger_msg->msg_data[0] = firmware->start_address;
+		msg->msg_data[0] = firmware->start_address;
 					/* Entry point */
-		trigger_msg->msg_data[1] = 6; /* Priority */
-		trigger_msg->msg_data[2] = 0x200; /* Stack size */
-		trigger_msg->msg_data[3] = 0; /* Parameter */
-		trigger_msg->msg_data[4] = 4; /* Task ID */
+		msg->msg_data[1] = 6; /* Priority */
+		msg->msg_data[2] = 0x200; /* Stack size */
+		msg->msg_data[3] = 0; /* Parameter */
+		msg->msg_data[4] = 4; /* Task ID */
 
-		rc = smscore_sendrequest_and_wait(coredev, trigger_msg,
-					trigger_msg->x_msg_header.msg_length,
+		rc = smscore_sendrequest_and_wait(coredev, msg,
+					msg->x_msg_header.msg_length,
 					&coredev->trigger_done);
 	} else {
 		SMS_INIT_MSG(&msg->x_msg_header, MSG_SW_RELOAD_EXEC_REQ,
diff --git a/drivers/media/common/siano/smscoreapi.h b/drivers/media/common/siano/smscoreapi.h
index 4a6b9f4c44ac..f8789ee0d554 100644
--- a/drivers/media/common/siano/smscoreapi.h
+++ b/drivers/media/common/siano/smscoreapi.h
@@ -624,9 +624,9 @@ struct sms_msg_data2 {
 	u32 msg_data[2];
 };
 
-struct sms_msg_data4 {
+struct sms_msg_data5 {
 	struct sms_msg_hdr x_msg_header;
-	u32 msg_data[4];
+	u32 msg_data[5];
 };
 
 struct sms_data_download {
-- 
2.27.0


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

* Re: [PATCH RESEND][next] media: siano: Fix out-of-bounds warnings in smscore_load_firmware_family2()
  2021-05-11 17:45 [PATCH RESEND][next] media: siano: Fix out-of-bounds warnings in smscore_load_firmware_family2() Gustavo A. R. Silva
@ 2021-06-01 22:12 ` Gustavo A. R. Silva
  2021-06-07 19:04   ` Gustavo A. R. Silva
  0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2021-06-01 22:12 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, linux-hardening, Kees Cook

Hi,

I've been pinging and resending this patch multiple times. Is there someone out there
that can give me some feedback on this, please?

I'm fine with adding this to my -next branch for 5.14 if no one cares. :)

Thanks
--
Gustavo

On 5/11/21 12:45, Gustavo A. R. Silva wrote:
> Rename struct sms_msg_data4 to sms_msg_data5 and increase the size of
> its msg_data array from 4 to 5 elements. Notice that at some point
> the 5th element of msg_data is being accessed in function
> smscore_load_firmware_family2():
> 
> 1006                 trigger_msg->msg_data[4] = 4; /* Task ID */
> 
> Also, there is no need for the object _trigger_msg_ of type struct
> sms_msg_data *, when _msg_ can be used, directly. Notice that msg_data
> in struct sms_msg_data is a one-element array, which causes multiple
> out-of-bounds warnings when accessing beyond its first element
> in function smscore_load_firmware_family2():
> 
>  992                 struct sms_msg_data *trigger_msg =                                                  
>  993                         (struct sms_msg_data *) msg;                                                
>  994                                                                                                     
>  995                 pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");                               
>  996                 SMS_INIT_MSG(&msg->x_msg_header,                                                    
>  997                                 MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,                                     
>  998                                 sizeof(struct sms_msg_hdr) +                                        
>  999                                 sizeof(u32) * 5);                                                   
> 1000                                                                                                     
> 1001                 trigger_msg->msg_data[0] = firmware->start_address;                                 
> 1002                                         /* Entry point */                                           
> 1003                 trigger_msg->msg_data[1] = 6; /* Priority */                                        
> 1004                 trigger_msg->msg_data[2] = 0x200; /* Stack size */                                  
> 1005                 trigger_msg->msg_data[3] = 0; /* Parameter */                                       
> 1006                 trigger_msg->msg_data[4] = 4; /* Task ID */ 
> 
> even when enough dynamic memory is allocated for _msg_:
> 
>  929         /* PAGE_SIZE buffer shall be enough and dma aligned */
>  930         msg = kmalloc(PAGE_SIZE, GFP_KERNEL | coredev->gfp_buf_flags);
> 
> but as _msg_ is casted to (struct sms_msg_data *):
> 
>  992                 struct sms_msg_data *trigger_msg =
>  993                         (struct sms_msg_data *) msg;
> 
> the out-of-bounds warnings are actually valid and should be addressed.
> 
> Fix this by declaring object _msg_ of type struct sms_msg_data5 *,
> which contains a 5-elements array, instead of just 4. And use
> _msg_ directly, instead of creating object trigger_msg.
> 
> This helps with the ongoing efforts to enable -Warray-bounds by fixing
> the following warnings:
> 
>   CC [M]  drivers/media/common/siano/smscoreapi.o
> drivers/media/common/siano/smscoreapi.c: In function ‘smscore_load_firmware_family2’:
> drivers/media/common/siano/smscoreapi.c:1003:24: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>  1003 |   trigger_msg->msg_data[1] = 6; /* Priority */
>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from drivers/media/common/siano/smscoreapi.c:12:
> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>   619 |  u32 msg_data[1];
>       |      ^~~~~~~~
> drivers/media/common/siano/smscoreapi.c:1004:24: warning: array subscript 2 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>  1004 |   trigger_msg->msg_data[2] = 0x200; /* Stack size */
>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from drivers/media/common/siano/smscoreapi.c:12:
> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>   619 |  u32 msg_data[1];
>       |      ^~~~~~~~
> drivers/media/common/siano/smscoreapi.c:1005:24: warning: array subscript 3 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>  1005 |   trigger_msg->msg_data[3] = 0; /* Parameter */
>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from drivers/media/common/siano/smscoreapi.c:12:
> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>   619 |  u32 msg_data[1];
>       |      ^~~~~~~~
> drivers/media/common/siano/smscoreapi.c:1006:24: warning: array subscript 4 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>  1006 |   trigger_msg->msg_data[4] = 4; /* Task ID */
>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
> In file included from drivers/media/common/siano/smscoreapi.c:12:
> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>   619 |  u32 msg_data[1];
>       |      ^~~~~~~~
> 
> Fixes: 018b0c6f8acb ("[media] siano: make load firmware logic to work with newer firmwares")
> Co-developed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> Hi,
> 
> We are about to be able to globally enable -Warray-bounds and,
> these are pretty much the last out-of-bounds warnings in linux-next.
> 
> Thanks
> 
>  drivers/media/common/siano/smscoreapi.c | 22 +++++++++-------------
>  drivers/media/common/siano/smscoreapi.h |  4 ++--
>  2 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/common/siano/smscoreapi.c b/drivers/media/common/siano/smscoreapi.c
> index 410cc3ac6f94..bceaf91faa15 100644
> --- a/drivers/media/common/siano/smscoreapi.c
> +++ b/drivers/media/common/siano/smscoreapi.c
> @@ -908,7 +908,7 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
>  					 void *buffer, size_t size)
>  {
>  	struct sms_firmware *firmware = (struct sms_firmware *) buffer;
> -	struct sms_msg_data4 *msg;
> +	struct sms_msg_data5 *msg;
>  	u32 mem_address,  calc_checksum = 0;
>  	u32 i, *ptr;
>  	u8 *payload = firmware->payload;
> @@ -989,24 +989,20 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
>  		goto exit_fw_download;
>  
>  	if (coredev->mode == DEVICE_MODE_NONE) {
> -		struct sms_msg_data *trigger_msg =
> -			(struct sms_msg_data *) msg;
> -
>  		pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");
>  		SMS_INIT_MSG(&msg->x_msg_header,
>  				MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,
> -				sizeof(struct sms_msg_hdr) +
> -				sizeof(u32) * 5);
> +				sizeof(*msg));
>  
> -		trigger_msg->msg_data[0] = firmware->start_address;
> +		msg->msg_data[0] = firmware->start_address;
>  					/* Entry point */
> -		trigger_msg->msg_data[1] = 6; /* Priority */
> -		trigger_msg->msg_data[2] = 0x200; /* Stack size */
> -		trigger_msg->msg_data[3] = 0; /* Parameter */
> -		trigger_msg->msg_data[4] = 4; /* Task ID */
> +		msg->msg_data[1] = 6; /* Priority */
> +		msg->msg_data[2] = 0x200; /* Stack size */
> +		msg->msg_data[3] = 0; /* Parameter */
> +		msg->msg_data[4] = 4; /* Task ID */
>  
> -		rc = smscore_sendrequest_and_wait(coredev, trigger_msg,
> -					trigger_msg->x_msg_header.msg_length,
> +		rc = smscore_sendrequest_and_wait(coredev, msg,
> +					msg->x_msg_header.msg_length,
>  					&coredev->trigger_done);
>  	} else {
>  		SMS_INIT_MSG(&msg->x_msg_header, MSG_SW_RELOAD_EXEC_REQ,
> diff --git a/drivers/media/common/siano/smscoreapi.h b/drivers/media/common/siano/smscoreapi.h
> index 4a6b9f4c44ac..f8789ee0d554 100644
> --- a/drivers/media/common/siano/smscoreapi.h
> +++ b/drivers/media/common/siano/smscoreapi.h
> @@ -624,9 +624,9 @@ struct sms_msg_data2 {
>  	u32 msg_data[2];
>  };
>  
> -struct sms_msg_data4 {
> +struct sms_msg_data5 {
>  	struct sms_msg_hdr x_msg_header;
> -	u32 msg_data[4];
> +	u32 msg_data[5];
>  };
>  
>  struct sms_data_download {
> 

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

* Re: [PATCH RESEND][next] media: siano: Fix out-of-bounds warnings in smscore_load_firmware_family2()
  2021-06-01 22:12 ` Gustavo A. R. Silva
@ 2021-06-07 19:04   ` Gustavo A. R. Silva
  0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2021-06-07 19:04 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, linux-hardening, Kees Cook

Hi all,

As I haven't received any response for this patch in almost three months[1] since I first
sent it out, I'm taking this in my -next[2] branch for v5.14.

Thanks
--
Gustavo

[1] https://lore.kernel.org/linux-hardening/20210311021947.GA129388@embeddedor/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp

On 6/1/21 17:12, Gustavo A. R. Silva wrote:
> Hi,
> 
> I've been pinging and resending this patch multiple times. Is there someone out there
> that can give me some feedback on this, please?
> 
> I'm fine with adding this to my -next branch for 5.14 if no one cares. :)
> 
> Thanks
> --
> Gustavo
> 
> On 5/11/21 12:45, Gustavo A. R. Silva wrote:
>> Rename struct sms_msg_data4 to sms_msg_data5 and increase the size of
>> its msg_data array from 4 to 5 elements. Notice that at some point
>> the 5th element of msg_data is being accessed in function
>> smscore_load_firmware_family2():
>>
>> 1006                 trigger_msg->msg_data[4] = 4; /* Task ID */
>>
>> Also, there is no need for the object _trigger_msg_ of type struct
>> sms_msg_data *, when _msg_ can be used, directly. Notice that msg_data
>> in struct sms_msg_data is a one-element array, which causes multiple
>> out-of-bounds warnings when accessing beyond its first element
>> in function smscore_load_firmware_family2():
>>
>>  992                 struct sms_msg_data *trigger_msg =                                                  
>>  993                         (struct sms_msg_data *) msg;                                                
>>  994                                                                                                     
>>  995                 pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");                               
>>  996                 SMS_INIT_MSG(&msg->x_msg_header,                                                    
>>  997                                 MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,                                     
>>  998                                 sizeof(struct sms_msg_hdr) +                                        
>>  999                                 sizeof(u32) * 5);                                                   
>> 1000                                                                                                     
>> 1001                 trigger_msg->msg_data[0] = firmware->start_address;                                 
>> 1002                                         /* Entry point */                                           
>> 1003                 trigger_msg->msg_data[1] = 6; /* Priority */                                        
>> 1004                 trigger_msg->msg_data[2] = 0x200; /* Stack size */                                  
>> 1005                 trigger_msg->msg_data[3] = 0; /* Parameter */                                       
>> 1006                 trigger_msg->msg_data[4] = 4; /* Task ID */ 
>>
>> even when enough dynamic memory is allocated for _msg_:
>>
>>  929         /* PAGE_SIZE buffer shall be enough and dma aligned */
>>  930         msg = kmalloc(PAGE_SIZE, GFP_KERNEL | coredev->gfp_buf_flags);
>>
>> but as _msg_ is casted to (struct sms_msg_data *):
>>
>>  992                 struct sms_msg_data *trigger_msg =
>>  993                         (struct sms_msg_data *) msg;
>>
>> the out-of-bounds warnings are actually valid and should be addressed.
>>
>> Fix this by declaring object _msg_ of type struct sms_msg_data5 *,
>> which contains a 5-elements array, instead of just 4. And use
>> _msg_ directly, instead of creating object trigger_msg.
>>
>> This helps with the ongoing efforts to enable -Warray-bounds by fixing
>> the following warnings:
>>
>>   CC [M]  drivers/media/common/siano/smscoreapi.o
>> drivers/media/common/siano/smscoreapi.c: In function ‘smscore_load_firmware_family2’:
>> drivers/media/common/siano/smscoreapi.c:1003:24: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>  1003 |   trigger_msg->msg_data[1] = 6; /* Priority */
>>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
>> In file included from drivers/media/common/siano/smscoreapi.c:12:
>> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>>   619 |  u32 msg_data[1];
>>       |      ^~~~~~~~
>> drivers/media/common/siano/smscoreapi.c:1004:24: warning: array subscript 2 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>  1004 |   trigger_msg->msg_data[2] = 0x200; /* Stack size */
>>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
>> In file included from drivers/media/common/siano/smscoreapi.c:12:
>> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>>   619 |  u32 msg_data[1];
>>       |      ^~~~~~~~
>> drivers/media/common/siano/smscoreapi.c:1005:24: warning: array subscript 3 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>  1005 |   trigger_msg->msg_data[3] = 0; /* Parameter */
>>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
>> In file included from drivers/media/common/siano/smscoreapi.c:12:
>> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>>   619 |  u32 msg_data[1];
>>       |      ^~~~~~~~
>> drivers/media/common/siano/smscoreapi.c:1006:24: warning: array subscript 4 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
>>  1006 |   trigger_msg->msg_data[4] = 4; /* Task ID */
>>       |   ~~~~~~~~~~~~~~~~~~~~~^~~
>> In file included from drivers/media/common/siano/smscoreapi.c:12:
>> drivers/media/common/siano/smscoreapi.h:619:6: note: while referencing ‘msg_data’
>>   619 |  u32 msg_data[1];
>>       |      ^~~~~~~~
>>
>> Fixes: 018b0c6f8acb ("[media] siano: make load firmware logic to work with newer firmwares")
>> Co-developed-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
>> ---
>> Hi,
>>
>> We are about to be able to globally enable -Warray-bounds and,
>> these are pretty much the last out-of-bounds warnings in linux-next.
>>
>> Thanks
>>
>>  drivers/media/common/siano/smscoreapi.c | 22 +++++++++-------------
>>  drivers/media/common/siano/smscoreapi.h |  4 ++--
>>  2 files changed, 11 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/media/common/siano/smscoreapi.c b/drivers/media/common/siano/smscoreapi.c
>> index 410cc3ac6f94..bceaf91faa15 100644
>> --- a/drivers/media/common/siano/smscoreapi.c
>> +++ b/drivers/media/common/siano/smscoreapi.c
>> @@ -908,7 +908,7 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
>>  					 void *buffer, size_t size)
>>  {
>>  	struct sms_firmware *firmware = (struct sms_firmware *) buffer;
>> -	struct sms_msg_data4 *msg;
>> +	struct sms_msg_data5 *msg;
>>  	u32 mem_address,  calc_checksum = 0;
>>  	u32 i, *ptr;
>>  	u8 *payload = firmware->payload;
>> @@ -989,24 +989,20 @@ static int smscore_load_firmware_family2(struct smscore_device_t *coredev,
>>  		goto exit_fw_download;
>>  
>>  	if (coredev->mode == DEVICE_MODE_NONE) {
>> -		struct sms_msg_data *trigger_msg =
>> -			(struct sms_msg_data *) msg;
>> -
>>  		pr_debug("sending MSG_SMS_SWDOWNLOAD_TRIGGER_REQ\n");
>>  		SMS_INIT_MSG(&msg->x_msg_header,
>>  				MSG_SMS_SWDOWNLOAD_TRIGGER_REQ,
>> -				sizeof(struct sms_msg_hdr) +
>> -				sizeof(u32) * 5);
>> +				sizeof(*msg));
>>  
>> -		trigger_msg->msg_data[0] = firmware->start_address;
>> +		msg->msg_data[0] = firmware->start_address;
>>  					/* Entry point */
>> -		trigger_msg->msg_data[1] = 6; /* Priority */
>> -		trigger_msg->msg_data[2] = 0x200; /* Stack size */
>> -		trigger_msg->msg_data[3] = 0; /* Parameter */
>> -		trigger_msg->msg_data[4] = 4; /* Task ID */
>> +		msg->msg_data[1] = 6; /* Priority */
>> +		msg->msg_data[2] = 0x200; /* Stack size */
>> +		msg->msg_data[3] = 0; /* Parameter */
>> +		msg->msg_data[4] = 4; /* Task ID */
>>  
>> -		rc = smscore_sendrequest_and_wait(coredev, trigger_msg,
>> -					trigger_msg->x_msg_header.msg_length,
>> +		rc = smscore_sendrequest_and_wait(coredev, msg,
>> +					msg->x_msg_header.msg_length,
>>  					&coredev->trigger_done);
>>  	} else {
>>  		SMS_INIT_MSG(&msg->x_msg_header, MSG_SW_RELOAD_EXEC_REQ,
>> diff --git a/drivers/media/common/siano/smscoreapi.h b/drivers/media/common/siano/smscoreapi.h
>> index 4a6b9f4c44ac..f8789ee0d554 100644
>> --- a/drivers/media/common/siano/smscoreapi.h
>> +++ b/drivers/media/common/siano/smscoreapi.h
>> @@ -624,9 +624,9 @@ struct sms_msg_data2 {
>>  	u32 msg_data[2];
>>  };
>>  
>> -struct sms_msg_data4 {
>> +struct sms_msg_data5 {
>>  	struct sms_msg_hdr x_msg_header;
>> -	u32 msg_data[4];
>> +	u32 msg_data[5];
>>  };
>>  
>>  struct sms_data_download {
>>

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

end of thread, other threads:[~2021-06-07 19:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-11 17:45 [PATCH RESEND][next] media: siano: Fix out-of-bounds warnings in smscore_load_firmware_family2() Gustavo A. R. Silva
2021-06-01 22:12 ` Gustavo A. R. Silva
2021-06-07 19:04   ` Gustavo A. R. Silva

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