[RESEND PATCH V4] staging: vchiq_arm: Add compatibility wrappers for ioctls

[Michael Zoran]

On Thu, 2017-03-02 at 10:45 -0800, Eric Anholt wrote:
> Michael Zoran <mzoran at crowfest.net> writes:
> 
> >  
> > +#if defined(CONFIG_COMPAT)
> > +static long
> > +vchiq_compat_ioctl_queue_message(struct file *file,
> > +				 unsigned int cmd,
> > +				 unsigned long arg)
> > +{
> > 
> I think inside of this block you should just check args32.count >
> MAX_ELEMENTS and return -EINVAL in that case, rather than silently
> not
> copying the elements.
> 

Sure, no problem on that.

> > +static long
> > +vchiq_compat_ioctl_await_completion(struct file *file,
> > +				    unsigned int cmd,
> > +				    unsigned long arg)
> > 
> Seems like instead of treating the user ioctl as having specified a
> count of 1 / msgbufcount of 1, we should throw -EINVAL if they
> specified
> something other than what we support for the compat path.
> 
> (this also means we don't need the weird bumping of msgbuf by
> msgbufcount - 1 in the copy_from_user above, right?)
> 
> There seem to be conditions in the real ioctl where msgbufcount
> doesn't
> get decremented.  Could we just get_user() the args->msgbufcount and
> copy that back out?
> 

That's a bit tricky to do because the actual code in user mode that I'm
trying to be compatible with doesn't actual specify a count of 1 /
msgbufcount of 1. In fact the old Broadcom graphics libraries and such
will use a big number here, so if I restrict things to 1 then all those
libraries will be broken which defeats the purpose of the wrappers.

The purpose of using 1 is to trick the native ioctl into only returning
 1 message at a time.  This tricks the user mode application into
thinking only 1 message was available, and like any good piece of
software it will try again if only some of the messages are available.

Another issue I'm trying to avoid is the application asynchronously
polling on returned data from an independent thread.  With the native
method it's possible that the application can start processing messages
even before the ioctl returns by polling the buffers. Forcing 1 message
at a time avoids that and avoids the broken error paths in the native
message which only happen if more then 1 message if requested at a
time.

BTW, this particular ioctl was by far the most complex one to get
working... I still want to eventual cleanup the old code, just the
compat approach will move things foreword until I can spend time fixing
the old in a way that people will agree with.

Let me know how I should proceed here.