Re: [PATCH 05/10] comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat

[Ian Abbott]

On 29/05/2020 01:35, Al Viro wrote:
> From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
>
> Just take copy_from_user() out of do_insn_ioctl() into the caller and
> have compat_insn() build a native version and pass it to do_insn_ioctl()
> directly.
>
> One difference from the previous commits is that the helper used to
> convert 32bit variant to native has two users - compat_insn() and
> compat_insnlist().  The latter will be converted in next commit;
> for now we simply split the helper in two variants - "userland 32bit
> to kernel native" and "userland 32bit to userland native".  The latter
> is renamed old get_compat_insn(); it will be gone in the next commit.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
>   drivers/staging/comedi/comedi_fops.c | 73 +++++++++++++++++++++++-------------
>   1 file changed, 46 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
> index d96dc85d8a98..ae0067ab5ead 100644
[snip]
> @@ -2244,10 +2241,13 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
>   				       (struct comedi_insnlist __user *)arg,
>   				       file);
>   		break;
> -	case COMEDI_INSN:
> -		rc = do_insn_ioctl(dev, (struct comedi_insn __user *)arg,
> -				   file);
> +	case COMEDI_INSN: {
> +		struct comedi_insn insn;
> +		if (copy_from_user(&insn, (void __user *)arg, sizeof(insn)))
> +			rc = -EFAULT;

Missing an 'else' here:

> +		rc = do_insn_ioctl(dev, &insn, file);
>   		break;
> +	}
>   	case COMEDI_POLL:

--
-=( Ian Abbott <abbotti@xxxxxxxxx> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-