Commit 84dcd594952bf9b95b3901516a61e57abdf54d62

Authored by Stephen Ware
Committed by Greg Kroah-Hartman
1 parent 71b7497c07

USB: fix up problems in the vtusb driver

Add range check on buffer sizes passed in from user space
(max is 8*PAGE_SIZE) which will work for the most common
spectrometers even at pages as small as 1K.

Add kref to vst device structure to preserve reference to the
usb object until we truly are done with it.

From: Stephen Ware <stephen.ware@eqware.net>
From: Dennis O'Brien <dennis.obrien@eqware.net>
Signed-off-by: Dennis O'Brien <dennis.obrien@eqware.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

Showing 1 changed file with 34 additions and 20 deletions Side-by-side Diff

drivers/usb/misc/vstusb.c
... ... @@ -59,6 +59,8 @@
59 59 #define USB_PRODUCT_LABPRO 0x0001
60 60 #define USB_PRODUCT_LABQUEST 0x0005
61 61  
  62 +#define VST_MAXBUFFER (64*1024)
  63 +
62 64 static struct usb_device_id id_table[] = {
63 65 { USB_DEVICE(USB_VENDOR_OCEANOPTICS, USB_PRODUCT_USB2000)},
64 66 { USB_DEVICE(USB_VENDOR_OCEANOPTICS, USB_PRODUCT_HR4000)},
... ... @@ -73,6 +75,7 @@
73 75 MODULE_DEVICE_TABLE(usb, id_table);
74 76  
75 77 struct vstusb_device {
  78 + struct kref kref;
76 79 struct mutex lock;
77 80 struct usb_device *usb_dev;
78 81 char present;
79 82  
... ... @@ -83,9 +86,18 @@
83 86 int wr_pipe;
84 87 int wr_timeout_ms;
85 88 };
  89 +#define to_vst_dev(d) container_of(d, struct vstusb_device, kref)
86 90  
87 91 static struct usb_driver vstusb_driver;
88 92  
  93 +static void vstusb_delete(struct kref *kref)
  94 +{
  95 + struct vstusb_device *vstdev = to_vst_dev(kref);
  96 +
  97 + usb_put_dev(vstdev->usb_dev);
  98 + kfree(vstdev);
  99 +}
  100 +
89 101 static int vstusb_open(struct inode *inode, struct file *file)
90 102 {
91 103 struct vstusb_device *vstdev;
... ... @@ -114,6 +126,9 @@
114 126 return -EBUSY;
115 127 }
116 128  
  129 + /* increment our usage count */
  130 + kref_get(&vstdev->kref);
  131 +
117 132 vstdev->isopen = 1;
118 133  
119 134 /* save device in the file's private structure */
... ... @@ -126,7 +141,7 @@
126 141 return 0;
127 142 }
128 143  
129   -static int vstusb_close(struct inode *inode, struct file *file)
  144 +static int vstusb_release(struct inode *inode, struct file *file)
130 145 {
131 146 struct vstusb_device *vstdev;
132 147  
133 148  
134 149  
... ... @@ -138,15 +153,13 @@
138 153 mutex_lock(&vstdev->lock);
139 154  
140 155 vstdev->isopen = 0;
141   - file->private_data = NULL;
142 156  
143   - /* if device is no longer present */
144   - if (!vstdev->present) {
145   - mutex_unlock(&vstdev->lock);
146   - kfree(vstdev);
147   - } else
148   - mutex_unlock(&vstdev->lock);
  157 + dev_dbg(&vstdev->usb_dev->dev, "%s: released\n", __func__);
149 158  
  159 + mutex_unlock(&vstdev->lock);
  160 +
  161 + kref_put(&vstdev->kref, vstusb_delete);
  162 +
150 163 return 0;
151 164 }
152 165  
... ... @@ -268,7 +281,7 @@
268 281 return -ENODEV;
269 282  
270 283 /* verify that we actually want to read some data */
271   - if (count == 0)
  284 + if ((count == 0) || (count > VST_MAXBUFFER))
272 285 return -EINVAL;
273 286  
274 287 /* lock this object */
... ... @@ -354,7 +367,7 @@
354 367 return -ENODEV;
355 368  
356 369 /* verify that we actually have some data to write */
357   - if (count == 0)
  370 + if ((count == 0) || (count > VST_MAXBUFFER))
358 371 return retval;
359 372  
360 373 /* lock this object */
... ... @@ -498,7 +511,7 @@
498 511  
499 512 case IOCTL_VSTUSB_SEND_PIPE:
500 513  
501   - if (usb_data.count == 0) {
  514 + if ((usb_data.count == 0) || (usb_data.count > VST_MAXBUFFER)) {
502 515 mutex_unlock(&vstdev->lock);
503 516 retval = -EINVAL;
504 517 goto exit;
... ... @@ -551,7 +564,7 @@
551 564 break;
552 565 case IOCTL_VSTUSB_RECV_PIPE:
553 566  
554   - if (usb_data.count == 0) {
  567 + if ((usb_data.count == 0) || (usb_data.count > VST_MAXBUFFER)) {
555 568 mutex_unlock(&vstdev->lock);
556 569 retval = -EINVAL;
557 570 goto exit;
... ... @@ -633,7 +646,7 @@
633 646 .unlocked_ioctl = vstusb_ioctl,
634 647 .compat_ioctl = vstusb_ioctl,
635 648 .open = vstusb_open,
636   - .release = vstusb_close,
  649 + .release = vstusb_release,
637 650 };
638 651  
639 652 static struct usb_class_driver usb_vstusb_class = {
... ... @@ -656,6 +669,10 @@
656 669 if (vstdev == NULL)
657 670 return -ENOMEM;
658 671  
  672 + /* must do usb_get_dev() prior to kref_init() since the kref_put()
  673 + * release function will do a usb_put_dev() */
  674 + usb_get_dev(dev);
  675 + kref_init(&vstdev->kref);
659 676 mutex_init(&vstdev->lock);
660 677  
661 678 i = dev->descriptor.bcdDevice;
... ... @@ -676,7 +693,7 @@
676 693 "%s: Not able to get a minor for this device.\n",
677 694 __func__);
678 695 usb_set_intfdata(intf, NULL);
679   - kfree(vstdev);
  696 + kref_put(&vstdev->kref, vstusb_delete);
680 697 return retval;
681 698 }
682 699  
683 700  
684 701  
... ... @@ -704,14 +721,11 @@
704 721  
705 722 usb_kill_anchored_urbs(&vstdev->submitted);
706 723  
707   - /* if the device is not opened, then we clean up right now */
708   - if (!vstdev->isopen) {
709   - mutex_unlock(&vstdev->lock);
710   - kfree(vstdev);
711   - } else
712   - mutex_unlock(&vstdev->lock);
  724 + mutex_unlock(&vstdev->lock);
713 725  
  726 + kref_put(&vstdev->kref, vstusb_delete);
714 727 }
  728 +
715 729 }
716 730  
717 731 static int vstusb_suspend(struct usb_interface *intf, pm_message_t message)