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

From: Ian Abbott
Date: Fri May 29 2020 - 06:11:50 EST

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()

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
@@ -2244,10 +2241,13 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
(struct comedi_insnlist __user *)arg,
- 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);
+ }

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