[PATCH RFC] staging: comedi: fix user/kernel space access of cmd->chanlist

[Ian Abbott]

On 2012-09-18 10:24, Ian Abbott wrote:
> On 2012-09-18 01:17, H Hartley Sweeten wrote:
>> The 'chanlist' in the comedi_cmd struct is an unsigned int __user
>> pointer.
>>
>> The do_cmd_ioctl() and do_cmdtest_ioctl() functions in comedi_fops
>> do a copy_from_user() to move the data from user space to kernel
>> space before passing the comedi_cmd to the comedi drivers.
>>
>> Unfortunately, the drivers then think 'chanlist' is still a
>> __user pointer since thats how the struct is defined.
>>
>> Make the 'chanlist' a union of both a __user and kernel pointer.
>> The do_cmd_*_ioctl() functions are the only ones that use the
>> __user pointer. All the drivers then use the kernel pointer to
>> access the chanlist.
>
> Personally, I'd rather get rid of the __user pointers in comedi.h and do
> the appropriate casting in the comedi core.

On further investigation, it seems that pointers in structs used by 
ioctls are generally tagged with __user in the kernel header files. 
Still, I don't think the low-level drivers should have to use union 
member access to get a kernel pointer.

The only other workaround I can think of is to have kernel versions of 
the ioctl data structures, e.g. `struct comedi_kcmd` shadowing `struct 
comedi_cmd` with the only difference being the lack of a `__user` tag on 
the pointers.  That would be a substantial patch.  Perhaps it could be 
broken down by temporarily defining macros such as

   #define comedi_kcmd comedi_cmd
   #define comedi_kinsn comedi_insn

etc. while the drivers are switched over to using `struct comedi_kcmd` 
instead of `struct comedi_cmd`, etc.  Once that is done, the new types 
can be defined for real (removing the macros) and the comedi core 
changed to copy the user types to the kernel types and vice versa.

This gives a clear demarcation between kernel types and user types 
without silly unions whose sole purpose is to keep `sparse` happy.  I 
expect the user types and kernel types to stay in sync, but they don't 
have to as long as conversions between the two are handled properly.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-