NAK: [SRU][X/E][PATCH] media: stv06xx: add missing descriptor sanity checks

Kleber Souza kleber.souza at canonical.com
Tue May 12 16:11:09 UTC 2020


On 30.04.20 00:42, Sultan Alsawaf wrote:
> From: Johan Hovold <johan at kernel.org>
> 
> CVE-2020-11609
> 
> Make sure to check that we have two alternate settings and at least one
> endpoint before accessing the second altsetting structure and
> dereferencing the endpoint arrays.
> 
> This specifically avoids dereferencing NULL-pointers or corrupting
> memory when a device does not have the expected descriptors.
> 
> Note that the sanity checks in stv06xx_start() and pb0100_start() are
> not redundant as the driver is mixing looking up altsettings by index
> and by number, which may not coincide.
> 
> Fixes: 8668d504d72c ("V4L/DVB (12082): gspca_stv06xx: Add support for st6422 bridge and sensor")
> Fixes: c0b33bdc5b8d ("[media] gspca-stv06xx: support bandwidth changing")
> Cc: stable <stable at vger.kernel.org>     # 2.6.31
> Cc: Hans de Goede <hdegoede at redhat.com>
> Signed-off-by: Johan Hovold <johan at kernel.org>
> Signed-off-by: Hans Verkuil <hverkuil-cisco at xs4all.nl>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei at kernel.org>
> (cherry picked from commit 485b06aadb933190f4bc44e006076bc27a23f205)
> Signed-off-by: Sultan Alsawaf <sultan.alsawaf at canonical.com>

This fix has already been applied to both Xenial and Eoan as part of upstream
stable updates:

https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1873852
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1871697


Thanks,
Kleber


> ---
>  drivers/media/usb/gspca/stv06xx/stv06xx.c     | 19 ++++++++++++++++++-
>  .../media/usb/gspca/stv06xx/stv06xx_pb0100.c  |  4 ++++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx.c b/drivers/media/usb/gspca/stv06xx/stv06xx.c
> index 79653d409951..95673fc0a99c 100644
> --- a/drivers/media/usb/gspca/stv06xx/stv06xx.c
> +++ b/drivers/media/usb/gspca/stv06xx/stv06xx.c
> @@ -282,6 +282,9 @@ static int stv06xx_start(struct gspca_dev *gspca_dev)
>  		return -EIO;
>  	}
>  
> +	if (alt->desc.bNumEndpoints < 1)
> +		return -ENODEV;
> +
>  	packet_size = le16_to_cpu(alt->endpoint[0].desc.wMaxPacketSize);
>  	err = stv06xx_write_bridge(sd, STV_ISO_SIZE_L, packet_size);
>  	if (err < 0)
> @@ -306,11 +309,21 @@ static int stv06xx_start(struct gspca_dev *gspca_dev)
>  
>  static int stv06xx_isoc_init(struct gspca_dev *gspca_dev)
>  {
> +	struct usb_interface_cache *intfc;
>  	struct usb_host_interface *alt;
>  	struct sd *sd = (struct sd *) gspca_dev;
>  
> +	intfc = gspca_dev->dev->actconfig->intf_cache[0];
> +
> +	if (intfc->num_altsetting < 2)
> +		return -ENODEV;
> +
> +	alt = &intfc->altsetting[1];
> +
> +	if (alt->desc.bNumEndpoints < 1)
> +		return -ENODEV;
> +
>  	/* Start isoc bandwidth "negotiation" at max isoc bandwidth */
> -	alt = &gspca_dev->dev->actconfig->intf_cache[0]->altsetting[1];
>  	alt->endpoint[0].desc.wMaxPacketSize =
>  		cpu_to_le16(sd->sensor->max_packet_size[gspca_dev->curr_mode]);
>  
> @@ -323,6 +336,10 @@ static int stv06xx_isoc_nego(struct gspca_dev *gspca_dev)
>  	struct usb_host_interface *alt;
>  	struct sd *sd = (struct sd *) gspca_dev;
>  
> +	/*
> +	 * Existence of altsetting and endpoint was verified in
> +	 * stv06xx_isoc_init()
> +	 */
>  	alt = &gspca_dev->dev->actconfig->intf_cache[0]->altsetting[1];
>  	packet_size = le16_to_cpu(alt->endpoint[0].desc.wMaxPacketSize);
>  	min_packet_size = sd->sensor->min_packet_size[gspca_dev->curr_mode];
> diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_pb0100.c b/drivers/media/usb/gspca/stv06xx/stv06xx_pb0100.c
> index 6d1007715ff7..ae382b3b5f7f 100644
> --- a/drivers/media/usb/gspca/stv06xx/stv06xx_pb0100.c
> +++ b/drivers/media/usb/gspca/stv06xx/stv06xx_pb0100.c
> @@ -185,6 +185,10 @@ static int pb0100_start(struct sd *sd)
>  	alt = usb_altnum_to_altsetting(intf, sd->gspca_dev.alt);
>  	if (!alt)
>  		return -ENODEV;
> +
> +	if (alt->desc.bNumEndpoints < 1)
> +		return -ENODEV;
> +
>  	packet_size = le16_to_cpu(alt->endpoint[0].desc.wMaxPacketSize);
>  
>  	/* If we don't have enough bandwidth use a lower framerate */
> 




More information about the kernel-team mailing list