On Tuesday, September 18, 2012 9:24 AM, Dan Carpenter wrote:
> On Tue, Sep 18, 2012 at 10:48:41AM -0500, H Hartley Sweeten wrote:
>> 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.
>>
>
> It's just four casts... At the state of do_cmd_ioctl() we do:
> user_chanlist = (unsigned int __user*)user_cmd.chanlist;
> and at the end we do:
> user_cmd.chanlist = (unsigned int __force *)user_chanlist;
>
> The same for do_cmdtest_ioctl().
>
> I like your naming and some the other cleanups you did. I would
> switch "user_cmd" to just "cmd" because otherwise it's a bit
> confusing.
I have been looking in the kernel for a similar issue. If looks like
fb.h had the same problem:
#ifdef __KERNEL__
#define FBIO_CURSOR _IOW('F', 0x08, struct fb_cursor_user)
#else
#define FBIO_CURSOR _IOW('F', 0x08, struct fb_cursor)
#endif
struct fb_cursor {
__u16 set;
__u16 enable;
__u16 rop;
const char *mask;
struct fbcurpos hot;
struct fb_image image;
};
struct fb_cursor_user {
__u16 set;
__u16 enable;
__u16 rop;
const char __user *mask;
struct fbcurpos hot;
struct fb_image_user image;
};
> You also swapped a kmalloc() for a kzalloc()... If that's a bugfix
> it should go in separately, otherwise it's not needed.
Oops... Sorry about that.
At this point I'm not sure what to do.
I think this still needs "fixed" but I'm confused as to what the
consensus is to "how" it should get fixed.
Regards,
Hartley
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
