linux-kernel - Re: [PATCH v2 14/19] [media] uvcvideo: prevent bounds-check bypass via speculative execution

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1624792.F9dcxCXkCx@avalon>
Date:   Tue, 07 Aug 2018 00:40:52 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        alan@...ux.intel.com, kernel-hardening@...ts.openwall.com,
        tglx@...utronix.de, Mauro Carvalho Chehab <mchehab@...nel.org>,
        torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
        Elena Reshetova <elena.reshetova@...el.com>,
        linux-media@...r.kernel.org
Subject: Re: [PATCH v2 14/19] [media] uvcvideo: prevent bounds-check bypass via speculative execution

Hi Dan,

Thank you for the patch.

On Friday, 12 January 2018 02:47:40 EEST Dan Williams wrote:
> Static analysis reports that 'index' may be a user controlled value that
> is used as a data dependency to read 'pin' from the
> 'selector->baSourceID' array. In order to avoid potential leaks of
> kernel memory values, block speculative execution of the instruction
> stream that could issue reads based on an invalid value of 'pin'.
> 
> Based on an original patch by Elena Reshetova.
> 
> Laurent notes:
> 
>     "...as this is nowhere close to being a fast path, I think we can close
>     this potential hole as proposed in the patch"
> 
> Cc: Mauro Carvalho Chehab <mchehab@...nel.org>
> Cc: linux-media@...r.kernel.org
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Signed-off-by: Elena Reshetova <elena.reshetova@...el.com>
> Signed-off-by: Dan Williams <dan.j.williams@...el.com>

What's the status of this series (and of this patch in particular) ?

> ---
>  drivers/media/usb/uvc/uvc_v4l2.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index 3e7e283a44a8..30ee200206ee 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -22,6 +22,7 @@
>  #include <linux/mm.h>
>  #include <linux/wait.h>
>  #include <linux/atomic.h>
> +#include <linux/nospec.h>
> 
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-ctrls.h>
> @@ -809,8 +810,12 @@ static int uvc_ioctl_enum_input(struct file *file, void
> *fh, const struct uvc_entity *selector = chain->selector;
>  	struct uvc_entity *iterm = NULL;
>  	u32 index = input->index;
> +	__u8 *elem = NULL;
>  	int pin = 0;
> 
> +	if (selector)
> +		elem = array_ptr(selector->baSourceID, index,
> +				selector->bNrInPins);
>  	if (selector == NULL ||
>  	    (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
>  		if (index != 0)
> @@ -820,8 +825,8 @@ static int uvc_ioctl_enum_input(struct file *file, void
> *fh, break;
>  		}
>  		pin = iterm->id;
> -	} else if (index < selector->bNrInPins) {
> -		pin = selector->baSourceID[index];
> +	} else if (elem) {
> +		pin = *elem;
>  		list_for_each_entry(iterm, &chain->entities, chain) {
>  			if (!UVC_ENTITY_IS_ITERM(iterm))
>  				continue;

-- 
Regards,

Laurent Pinchart