hidraw: Push down BKL into ioctl handler
PATCH 1/1 hidraw: Push down BKL into ioctl handler
In this case I simply wrapped it as code review suggests the locking already terminally broken and I didn't want to make it first. See added comment
Signed-off-by Alan Cox <[email protected]>
drivers/hid/hidraw.c +25 -22
@@ -30,6 +30,7 @@#include <linux/major.h> #include <linux/hid.h> #include <linux/mutex.h> +#include <linux/smp_lock.h> #include <linux/hidraw.h>
@@ -104,6 +105,7 @@ out:static ssize_t hidraw_write (struct file *file,const char __user *buffer,size_t count,loff_t *ppos) {unsigned int minor =iminor (file->f_path.dentry->d_inode); +/* FIXME: What stops hidraw_table going NULL */ struct hid_device *dev = hidraw_table[minor]->hid;__u8 *buf;int ret =0 ;
@@ -211,35 +213,38 @@ static int hidraw_release(struct inode * inode, struct file * file)return 0 ; } -static int hidraw_ioctl (struct inode *inode,struct file *file,unsigned int cmd,unsigned long arg) +static long hidraw_ioctl (struct file *file,unsigned int cmd, +unsigned long arg) { +struct inode *inode = file->f_path.dentry->d_inode;unsigned int minor =iminor (inode); +long ret =0 ; +/* FIXME: What stops hidraw_table going NULL */ struct hidraw *dev = hidraw_table[minor];void __user *user_arg = (void __user *) arg; +lock_kernel ();switch (cmd) {case HIDIOCGRDESCSIZE :if (put_user (dev->hid->rsize, (int __user *)arg)) -return -EFAULT ; -return 0 ; + ret = -EFAULT ; +break ;case HIDIOCGRDESC : {__u32 len;if (get_user (len, (int __user *)arg)) -return -EFAULT ; - -if (len >HID_MAX_DESCRIPTOR_SIZE -1 ) -return -EINVAL ; - -if (copy_to_user (user_arg + offsetof( -struct hidraw_report_descriptor , - value[0]), - dev->hid->rdesc, - min(dev->hid->rsize, len))) -return -EFAULT ; -return 0 ; + ret = -EFAULT ; +else if (len >HID_MAX_DESCRIPTOR_SIZE -1 ) + ret = -EINVAL ; +else if (copy_to_user (user_arg + offsetof( +struct hidraw_report_descriptor , + value[0]), + dev->hid->rdesc, + min(dev->hid->rsize, len))) + ret = -EFAULT ; +break ; }case HIDIOCGRAWINFO : {
@@ -249,15 +254,13 @@ static int hidraw_ioctl(struct inode *inode, struct file *file, unsigned int cmd dinfo.vendor = dev->hid->vendor; dinfo.product = dev->hid->product;if (copy_to_user (user_arg, &dinfo,sizeof (dinfo))) -return -EFAULT ; - -return 0 ; + ret = -EFAULT ; +break ; }default : -printk (KERN_EMERG "hidraw: unsupported ioctl() %x\n" , - cmd); + ret = -ENOTTY ; } -return -EINVAL ; +return ret; }static const struct file_operations hidraw_ops = {
@@ -267,7 +270,7 @@ static const struct file_operations hidraw_ops = { .poll = hidraw_poll, .open = hidraw_open, .release = hidraw_release, - .ioctl = hidraw_ioctl, + .unlocked_ioctl = hidraw_ioctl, };void hidraw_report_event (struct hid_device *hid,u8 *data,int len)
Quoted 3 lines · Alan Cox
In this case I simply wrapped it as code review suggests the locking already terminally broken and I didn't want to make it first. See added comment
Thanks a lot, I will apply your patch and fix the synchronization on accessing hidraw_table[] on top of it.
Signature · 1 line
Jiri Kosina