| From 1c81c69b140718a0eba9f6dda5bbe0412052afcd Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 10 Mar 2021 19:40:43 -0600 |
| Subject: media: siano: Fix out-of-bounds warnings in |
| smscore_load_firmware_family2() |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Gustavo A. R. Silva <gustavoars@kernel.org> |
| |
| [ Upstream commit 13dfead49db07225335d4f587a560a2210391a1a ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| 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 c1511094fdc7..b735e2370137 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 b3b793b5caf3..16c45afabc53 100644 |
| --- a/drivers/media/common/siano/smscoreapi.h |
| +++ b/drivers/media/common/siano/smscoreapi.h |
| @@ -629,9 +629,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.30.2 |
| |