media: gspca: Add bounds checking to firmware parser

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: stable@vger.kernel.org
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Hans Verkuil <hverkuil@xs4all.nl>

diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index d98343fd33fe34b59b91eaba7229f2ee7fd78628..91e177aa8136fd121059c3f0857787608de8023e 100644 (file)
--- 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;
        }