linux-kernel - Re: [PATCH 04/20] media: uvc: uvc_v4l2.c: add temp variable for list iteration

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] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 8 Aug 2020 23:45:17 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     "Daniel W. S. Almeida" <dwlsalmeida@...il.com>
Cc:     skhan@...uxfoundation.org,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/20] media: uvc: uvc_v4l2.c: add temp variable for list
 iteration

Hi Daniel,

Thank you for the patch.

On Fri, Aug 07, 2020 at 05:35:31AM -0300, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <dwlsalmeida@...il.com>
> 
> Fixes the following coccinelle reports:
> 
> drivers/media/usb/uvc/uvc_v4l2.c:840:8-13:
> ERROR: invalid reference to the index variable of the iterator on line 836
> 
> drivers/media/usb/uvc/uvc_v4l2.c:851:5-10:
> ERROR: invalid reference to the index variable of the iterator on line 843
> 
> drivers/media/usb/uvc/uvc_v4l2.c:851:22-27:
> ERROR: invalid reference to the index variable of the iterator on line 843
> 
> Byy introducing a temporary variable for list iteration.
> 
> Found using - Coccinelle (http://coccinelle.lip6.fr)
> 
> Signed-off-by: Daniel W. S. Almeida <dwlsalmeida@...il.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 0335e69b70ab..7205ef13c2e1 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -826,6 +826,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
>  	struct uvc_video_chain *chain = handle->chain;
>  	const struct uvc_entity *selector = chain->selector;
>  	struct uvc_entity *iterm = NULL;
> +	struct uvc_entity *cursor = NULL;
>  	u32 index = input->index;
>  	int pin = 0;
>  
> @@ -833,18 +834,22 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
>  	    (chain->dev->quirks & UVC_QUIRK_IGNORE_SELECTOR_UNIT)) {
>  		if (index != 0)
>  			return -EINVAL;
> -		list_for_each_entry(iterm, &chain->entities, chain) {
> -			if (UVC_ENTITY_IS_ITERM(iterm))
> +		list_for_each_entry(cursor, &chain->entities, chain) {
> +			if (UVC_ENTITY_IS_ITERM(cursor)){
> +				iterm = cursor;
>  				break;
> +			}
>  		}

If no entity is found by this loop the next line will dereference a NULL
pointer. There's a similar issue below.

I think the issues reported by coccinelle are false positives, as every
chain is guaranteed to have the proper input terminals (ITERM). This is
verified when constructing the chain at probe time.

>  		pin = iterm->id;
>  	} else if (index < selector->bNrInPins) {
>  		pin = selector->baSourceID[index];
> -		list_for_each_entry(iterm, &chain->entities, chain) {
> -			if (!UVC_ENTITY_IS_ITERM(iterm))
> +		list_for_each_entry(cursor, &chain->entities, chain) {
> +			if (!UVC_ENTITY_IS_ITERM(cursor))
>  				continue;
> -			if (iterm->id == pin)
> +			if (cursor->id == pin) {
> +				iterm = cursor;
>  				break;
> +			}
>  		}
>  	}
>  
> @@ -1519,4 +1524,3 @@ const struct v4l2_file_operations uvc_fops = {
>  	.get_unmapped_area = uvc_v4l2_get_unmapped_area,
>  #endif
>  };
> -

-- 
Regards,

Laurent Pinchart