[PATCH for v5.18] v4l2-ioctl.c: fix incorrect error path

[PATCH for v5.18] v4l2-ioctl.c: fix incorrect error path

[Hans Verkuil]

If allocating array_buf fails, or copying data from userspace into that
buffer fails, then just free memory and return the error. Don't attempt
to call video_put_user() since there is no point.

If writing the array back to userspace fails, then don't go to out_array_args,
instead just continue with the regular code that just returns the error
unless 'always_copy' is set. That flag was just completely ignored when
writing back the array data.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 96e307fe3aab..92433234c430 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -3311,7 +3311,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
 		array_buf = kvmalloc(array_size, GFP_KERNEL);
 		err = -ENOMEM;
 		if (array_buf == NULL)
-			goto out_array_args;
+			goto out;
 		err = -EFAULT;
 		if (in_compat_syscall())
 			err = v4l2_compat_get_array_args(file, array_buf,
@@ -3321,7 +3321,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
 			err = copy_from_user(array_buf, user_ptr, array_size) ?
 								-EFAULT : 0;
 		if (err)
-			goto out_array_args;
+			goto out;
 		*kernel_ptr = array_buf;
 	}

@@ -3353,7 +3353,6 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
 		} else if (copy_to_user(user_ptr, array_buf, array_size)) {
 			err = -EFAULT;
 		}
-		goto out_array_args;
 	}
 	/*
 	 * Some ioctls can return an error, but still have valid

Re: [PATCH for v5.18] v4l2-ioctl.c: fix incorrect error path

[Hans Verkuil]

On 21/03/2022 09:47, Hans Verkuil wrote:
> If allocating array_buf fails, or copying data from userspace into that
> buffer fails, then just free memory and return the error. Don't attempt
> to call video_put_user() since there is no point.
> 
> If writing the array back to userspace fails, then don't go to out_array_args,
> instead just continue with the regular code that just returns the error
> unless 'always_copy' is set. That flag was just completely ignored when
> writing back the array data.

Ignore this patch, it needs a bit more work.

	Hans

> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 96e307fe3aab..92433234c430 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -3311,7 +3311,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
>  		array_buf = kvmalloc(array_size, GFP_KERNEL);
>  		err = -ENOMEM;
>  		if (array_buf == NULL)
> -			goto out_array_args;
> +			goto out;
>  		err = -EFAULT;
>  		if (in_compat_syscall())
>  			err = v4l2_compat_get_array_args(file, array_buf,
> @@ -3321,7 +3321,7 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
>  			err = copy_from_user(array_buf, user_ptr, array_size) ?
>  								-EFAULT : 0;
>  		if (err)
> -			goto out_array_args;
> +			goto out;
>  		*kernel_ptr = array_buf;
>  	}
> 
> @@ -3353,7 +3353,6 @@ video_usercopy(struct file *file, unsigned int orig_cmd, unsigned long arg,
>  		} else if (copy_to_user(user_ptr, array_buf, array_size)) {
>  			err = -EFAULT;
>  		}
> -		goto out_array_args;
>  	}
>  	/*
>  	 * Some ioctls can return an error, but still have valid
>