On Wed May 28 23:22:14 2025 +0300, Dan Carpenter wrote:
> This sd_init() function reads the firmware. The firmware data holds a
> series of records and the function reads each record and sends the data
> to the device. The request_ihex_firmware() function
> calls ihex_validate_fw() which ensures that the total length of all the
> records won't read out of bounds of the fw->data[].
>
> However, a potential issue is if there is a single very large
> record (larger than PAGE_SIZE) and that would result in memory
> corruption. Generally we trust the firmware, but it's always better to
> double check.
>
> Fixes: 49b61ec9b5af ("[media] gspca: Add new vicam subdriver")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Dan Carpenter <dan.carpen...@linaro.org>
> Signed-off-by: Hans Verkuil <hverk...@xs4all.nl>
Patch committed.
Thanks,
Hans Verkuil
drivers/media/usb/gspca/vicam.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
---
diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index d98343fd33fe..91e177aa8136 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -227,6 +227,7 @@ static int sd_init(struct gspca_dev *gspca_dev)
const struct ihex_binrec *rec;
const struct firmware *fw;
u8 *firmware_buf;
+ int len;
ret = request_ihex_firmware(&fw, VICAM_FIRMWARE,
&gspca_dev->dev->dev);
@@ -241,9 +242,14 @@ static int sd_init(struct gspca_dev *gspca_dev)
goto exit;
}
for (rec = (void *)fw->data; rec; rec = ihex_next_binrec(rec)) {
- memcpy(firmware_buf, rec->data, be16_to_cpu(rec->len));
+ len = be16_to_cpu(rec->len);
+ if (len > PAGE_SIZE) {
+ ret = -EINVAL;
+ break;
+ }
+ memcpy(firmware_buf, rec->data, len);
ret = vicam_control_msg(gspca_dev, 0xff, 0, 0, firmware_buf,
- be16_to_cpu(rec->len));
+ len);
if (ret < 0)
break;
}
