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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux