[PATCH] staging: vc04_services: rework ioctl code path

[Michael Zoran]

On Wed, 2016-11-09 at 23:51 +0100, Arnd Bergmann wrote:
> On Wednesday, November 9, 2016 2:41:42 PM CET Michael Zoran wrote:
> > > >     Arnd
> > > 
> > > That I have absolutely no problem changing.  I'll go ahead and
> > > change
> > > it and resubmit the patch.
> > > > 
> > 
> > Actually, I'm looking at this a bit more and I'm noticing some
> > interesting things.
> > 
> > If something like compat_ptr is used, why can the ioctl.h header be
> > included twice with different #defines to generate both the 32 bit
> > and
> > 64 bit cases.
> 
> I can't see how that would work, as you have to change the macro
> names as well.
> 

I think the C preprocessor allows macros to reference other macros.  So
I can easily create a new set of macros that are conditionally defined
to map to either compat_ptr in one case or foo __user * in the other
case.

Perhaps I should look into this more.

> > Also, I'm think more about the kernel pointer idea.  One
> > complication
> > I'm thinking is of the case of IN/OUT data.   How does it unwind
> > the
> > operation on a failure of the final copy back to userland?
> > 
> > On thing I'm just brainstorming about is what can't it "lock" the
> > page(s) for the userland pointer in ram until the end of the ioctl
> > so
> > that the copy back from the kernel can't ever or can't very easily
> > fail?
> 
> The normal approach is to return without copying back if something
> fails before the copy_to_user(), but return -EFAULT if the copy
> itself fails for a _IOWR or _IOR command.
> 
> 	Arnd

That makes sense.  But one of the ioctls needs to return an array of
data items each of which can fail on the copy.  I'm wondering if
perhaps most of the errors can be avoided by the page locking scheme.