On Tuesday, September 18, 2012 5:11 AM, Ian Abbott wrote: > 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 What about just renaming the user version of the struct and then creating the 'shadowing' kernel struct without the __user tag? This would have the least impact on the code. The drivers would not be changed at all. Only comedi_fops deals with the __user version. I'll post a patch shortly for another RFC. Regards, Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
