On Mon, 2017-03-06 at 18:22 +0300, Dan Carpenter wrote:
> > +
> > +struct vchiq_completion_data32 {
> > + VCHIQ_REASON_T reason;
> > + compat_uptr_t header;
> > + compat_uptr_t service_userdata;
> > + compat_uptr_t bulk_userdata;
> > +};
> > +
> > +struct vchiq_await_completion32 {
> > + unsigned int count;
> > + compat_uptr_t buf;
> > + unsigned int msgbufsize;
> > + unsigned int msgbufcount; /* IN/OUT */
> > + compat_uptr_t msgbufs;
> > +};
> > +
> > +#define VCHIQ_IOC_AWAIT_COMPLETION32 \
> > + _IOWR(VCHIQ_IOC_MAGIC, 7, struct vchiq_await_completion32)
> > +
> > +static long
> > +vchiq_compat_ioctl_await_completion(struct file *file,
> > + unsigned int cmd,
> > + unsigned long arg)
> > +{
> > + VCHIQ_AWAIT_COMPLETION_T *args;
> > + VCHIQ_COMPLETION_DATA_T *completion;
> > + VCHIQ_COMPLETION_DATA_T completiontemp;
> > + struct vchiq_await_completion32 args32;
> > + struct vchiq_completion_data32 completion32;
> > + unsigned int *msgbufcount32;
> > + compat_uptr_t msgbuf32;
> > + void *msgbuf;
> > + void **msgbufptr;
> > + long ret;
> > +
> > + args = compat_alloc_user_space(sizeof(*args) +
> > + sizeof(*completion) +
> > + sizeof(*msgbufptr));
> > + if (!args)
> > + return -EFAULT;
> > +
> > + completion = (VCHIQ_COMPLETION_DATA_T *)(args + 1);
> > + msgbufptr = (void __user **)(completion + 1);
> > +
> > + if (copy_from_user(&args32,
> > + (struct vchiq_completion_data32 *)arg,
> > + sizeof(args32)))
> > + return -EFAULT;
> > +
> > + if (put_user(args32.count, &args->count) ||
> > + put_user(compat_ptr(args32.buf), &args->buf) ||
> > + put_user(args32.msgbufsize, &args->msgbufsize) ||
> > + put_user(args32.msgbufcount, &args->msgbufcount) ||
> > + put_user(compat_ptr(args32.msgbufs), &args->msgbufs))
> > + return -EFAULT;
> > +
> > + if (!args32.count || !args32.buf || !args32.msgbufcount)
> > + return vchiq_ioctl(file,
> > + VCHIQ_IOC_AWAIT_COMPLETION,
> > + (unsigned long)args);
> > +
> > + if (copy_from_user(&msgbuf32,
> > + compat_ptr(args32.msgbufs) +
> > + (sizeof(compat_uptr_t) *
> > + (args32.msgbufcount - 1)),
> > + sizeof(msgbuf32)))
> > + return -EFAULT;
> > +
> > + msgbuf = compat_ptr(msgbuf32);
> > +
> > + if (copy_to_user(msgbufptr,
> > + &msgbuf,
> > + sizeof(msgbuf)))
> > + return -EFAULT;
> > +
> > + if (copy_to_user(&args->msgbufs,
> > + &msgbufptr,
> > + sizeof(msgbufptr)))
> > + return -EFAULT;
> > +
> > + if (put_user(1U, &args->count) ||
> > + put_user(completion, &args->buf) ||
> > + put_user(1U, &args->msgbufcount))
> > + return -EFAULT;
> > +
> > + ret = vchiq_ioctl(file,
> > + VCHIQ_IOC_AWAIT_COMPLETION,
> > + (unsigned long)args);
> > +
> > + if (ret <= 0)
>
> Really?
>
Believe it or not, this code actually does work. It's the original
code that's convoluted.
By forcing 1 for args->count and args->msgbufcount, it avoids all the
broken code paths in the original code.
Here is a brief summary of what the orignal code returns:
0: No messages where available.
>0: Number of messages that were availably but never more then args-
>count.
<0: An error occured, but only if only 1 message was available.
Otherwise return the count of available messages.
> > + return ret;
> > +
> > + if (copy_from_user(&completiontemp, completion,
> > sizeof(*completion)))
> > + return -EFAULT;
> > +
> > + completion32.reason = completiontemp.reason;
> > + completion32.header =
> > ptr_to_compat(completiontemp.header);
> > + completion32.service_userdata =
> > + ptr_to_compat(completiontemp.service_userdata);
> > + completion32.bulk_userdata =
> > + ptr_to_compat(completiontemp.bulk_userdata);
> > +
> > + if (copy_to_user(compat_ptr(args32.buf),
> > + &completion32,
> > + sizeof(completion32)))
> > + return -EFAULT;
> > +
> > + args32.msgbufcount--;
> > +
> > + msgbufcount32 =
> > + &((struct vchiq_await_completion32 __user *)arg)-
> > >msgbufcount;
> > +
> > + if (copy_to_user(msgbufcount32,
> > + &args32.msgbufcount,
> > + sizeof(args32.msgbufcount)))
> > + return -EFAULT;
> > +
> > + return ret;
>
> return 0;
>
The other fixes are very easy to make and I have no problem changes
those.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
