linux-kernel - Re: [PATCH 05/10] comedi: get rid of compat_alloc_user_space() mess in COMEDI

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:   Fri, 29 May 2020 11:05:34 +0100
From:   Ian Abbott <abbotti@....co.uk>
To:     Al Viro <viro@...IV.linux.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 05/10] comedi: get rid of compat_alloc_user_space() mess
 in COMEDI_INSN compat

On 29/05/2020 01:35, Al Viro wrote:
> From: Al Viro <viro@...iv.linux.org.uk>
> 
> 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@...iv.linux.org.uk>
> ---
>   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@....co.uk> || 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. )=-