[PATCH 0/3] ihex: add some bounds checking to firmware parsing

[PATCH 0/3] ihex: add some bounds checking to firmware parsing

[Dan Carpenter]

These three patches go to different subsystems so hopefully the
individual maintainers can apply them.  I can resend them as
individual patches if that's easier.

The ihex firmware code is a list of records of various lengths.  The
ihex code ensures that total length of the records doesn't read
beyond the end of the fw->data[], however the parsers need to check
that individual records are not too large.

Dan Carpenter (3):
  media: gspca: Add bounds checking to firmware parser
  watchdog: ziirave_wdt: check record length in ziirave_firm_verify()
  Input: ims-pcu - Check record size in ims_pcu_flash_firmware()

 drivers/input/misc/ims-pcu.c    |  6 ++++++
 drivers/media/usb/gspca/vicam.c | 10 ++++++++--
 drivers/watchdog/ziirave_wdt.c  |  3 +++
 3 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.47.2

[PATCH 2/3] watchdog: ziirave_wdt: check record length in ziirave_firm_verify()

[Dan Carpenter]

The "rec->len" value comes from the firmware.  We generally do
trust firmware, but it's always better to double check.  If
the length value is too large it would lead to memory corruption
when we set "data[i] = ret;"

Fixes: 217209db0204 ("watchdog: ziirave_wdt: Add support to upload the firmware.")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/watchdog/ziirave_wdt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
index fcc1ba02e75b..5c6e3fa001d8 100644
--- a/drivers/watchdog/ziirave_wdt.c
+++ b/drivers/watchdog/ziirave_wdt.c
@@ -302,6 +302,9 @@ static int ziirave_firm_verify(struct watchdog_device *wdd,
 		const u16 len = be16_to_cpu(rec->len);
 		const u32 addr = be32_to_cpu(rec->addr);
 
+		if (len > sizeof(data))
+			return -EINVAL;
+
 		if (ziirave_firm_addr_readonly(addr))
 			continue;
 
-- 
2.47.2

Re: [PATCH 2/3] watchdog: ziirave_wdt: check record length in ziirave_firm_verify()

[Guenter Roeck]

On 5/28/25 13:22, Dan Carpenter wrote:
> The "rec->len" value comes from the firmware.  We generally do
> trust firmware, but it's always better to double check.  If
> the length value is too large it would lead to memory corruption
> when we set "data[i] = ret;"
> 
> Fixes: 217209db0204 ("watchdog: ziirave_wdt: Add support to upload the firmware.")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Guenetr Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/ziirave_wdt.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/watchdog/ziirave_wdt.c b/drivers/watchdog/ziirave_wdt.c
> index fcc1ba02e75b..5c6e3fa001d8 100644
> --- a/drivers/watchdog/ziirave_wdt.c
> +++ b/drivers/watchdog/ziirave_wdt.c
> @@ -302,6 +302,9 @@ static int ziirave_firm_verify(struct watchdog_device *wdd,
>   		const u16 len = be16_to_cpu(rec->len);
>   		const u32 addr = be32_to_cpu(rec->addr);
>   
> +		if (len > sizeof(data))
> +			return -EINVAL;
> +
>   		if (ziirave_firm_addr_readonly(addr))
>   			continue;
>