Commit 89dec01b authored by Dmitry Torokhov's avatar Dmitry Torokhov Committed by Mauro Carvalho Chehab

V4L/DVB (12489): pwc - fix few use-after-free and memory leaks

I just happen to peek inside the PWC driver and did not like what I saw
there. Please consider applying the patch below.
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 35c3057b
...@@ -1057,7 +1057,8 @@ static int pwc_create_sysfs_files(struct video_device *vdev) ...@@ -1057,7 +1057,8 @@ static int pwc_create_sysfs_files(struct video_device *vdev)
goto err; goto err;
if (pdev->features & FEATURE_MOTOR_PANTILT) { if (pdev->features & FEATURE_MOTOR_PANTILT) {
rc = device_create_file(&vdev->dev, &dev_attr_pan_tilt); rc = device_create_file(&vdev->dev, &dev_attr_pan_tilt);
if (rc) goto err_button; if (rc)
goto err_button;
} }
return 0; return 0;
...@@ -1072,6 +1073,7 @@ static int pwc_create_sysfs_files(struct video_device *vdev) ...@@ -1072,6 +1073,7 @@ static int pwc_create_sysfs_files(struct video_device *vdev)
static void pwc_remove_sysfs_files(struct video_device *vdev) static void pwc_remove_sysfs_files(struct video_device *vdev)
{ {
struct pwc_device *pdev = video_get_drvdata(vdev); struct pwc_device *pdev = video_get_drvdata(vdev);
if (pdev->features & FEATURE_MOTOR_PANTILT) if (pdev->features & FEATURE_MOTOR_PANTILT)
device_remove_file(&vdev->dev, &dev_attr_pan_tilt); device_remove_file(&vdev->dev, &dev_attr_pan_tilt);
device_remove_file(&vdev->dev, &dev_attr_button); device_remove_file(&vdev->dev, &dev_attr_button);
...@@ -1229,13 +1231,11 @@ static void pwc_cleanup(struct pwc_device *pdev) ...@@ -1229,13 +1231,11 @@ static void pwc_cleanup(struct pwc_device *pdev)
video_unregister_device(pdev->vdev); video_unregister_device(pdev->vdev);
#ifdef CONFIG_USB_PWC_INPUT_EVDEV #ifdef CONFIG_USB_PWC_INPUT_EVDEV
if (pdev->button_dev) { if (pdev->button_dev)
input_unregister_device(pdev->button_dev); input_unregister_device(pdev->button_dev);
input_free_device(pdev->button_dev);
kfree(pdev->button_dev->phys);
pdev->button_dev = NULL;
}
#endif #endif
kfree(pdev);
} }
/* Note that all cleanup is done in the reverse order as in _open */ /* Note that all cleanup is done in the reverse order as in _open */
...@@ -1281,8 +1281,6 @@ static int pwc_video_close(struct file *file) ...@@ -1281,8 +1281,6 @@ static int pwc_video_close(struct file *file)
PWC_DEBUG_OPEN("<< video_close() vopen=%d\n", pdev->vopen); PWC_DEBUG_OPEN("<< video_close() vopen=%d\n", pdev->vopen);
} else { } else {
pwc_cleanup(pdev); pwc_cleanup(pdev);
/* Free memory (don't set pdev to 0 just yet) */
kfree(pdev);
/* search device_hint[] table if we occupy a slot, by any chance */ /* search device_hint[] table if we occupy a slot, by any chance */
for (hint = 0; hint < MAX_DEV_HINTS; hint++) for (hint = 0; hint < MAX_DEV_HINTS; hint++)
if (device_hint[hint].pdev == pdev) if (device_hint[hint].pdev == pdev)
...@@ -1499,13 +1497,10 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1499,13 +1497,10 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
struct usb_device *udev = interface_to_usbdev(intf); struct usb_device *udev = interface_to_usbdev(intf);
struct pwc_device *pdev = NULL; struct pwc_device *pdev = NULL;
int vendor_id, product_id, type_id; int vendor_id, product_id, type_id;
int i, hint, rc; int hint, rc;
int features = 0; int features = 0;
int video_nr = -1; /* default: use next available device */ int video_nr = -1; /* default: use next available device */
char serial_number[30], *name; char serial_number[30], *name;
#ifdef CONFIG_USB_PWC_INPUT_EVDEV
char *phys = NULL;
#endif
vendor_id = le16_to_cpu(udev->descriptor.idVendor); vendor_id = le16_to_cpu(udev->descriptor.idVendor);
product_id = le16_to_cpu(udev->descriptor.idProduct); product_id = le16_to_cpu(udev->descriptor.idProduct);
...@@ -1757,8 +1752,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1757,8 +1752,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
pdev->vframes = default_fps; pdev->vframes = default_fps;
strcpy(pdev->serial, serial_number); strcpy(pdev->serial, serial_number);
pdev->features = features; pdev->features = features;
if (vendor_id == 0x046D && product_id == 0x08B5) if (vendor_id == 0x046D && product_id == 0x08B5) {
{
/* Logitech QuickCam Orbit /* Logitech QuickCam Orbit
The ranges have been determined experimentally; they may differ from cam to cam. The ranges have been determined experimentally; they may differ from cam to cam.
Also, the exact ranges left-right and up-down are different for my cam Also, the exact ranges left-right and up-down are different for my cam
...@@ -1780,8 +1774,8 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1780,8 +1774,8 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
pdev->vdev = video_device_alloc(); pdev->vdev = video_device_alloc();
if (!pdev->vdev) { if (!pdev->vdev) {
PWC_ERROR("Err, cannot allocate video_device struture. Failing probe."); PWC_ERROR("Err, cannot allocate video_device struture. Failing probe.");
kfree(pdev); rc = -ENOMEM;
return -ENOMEM; goto err_free_mem;
} }
memcpy(pdev->vdev, &pwc_template, sizeof(pwc_template)); memcpy(pdev->vdev, &pwc_template, sizeof(pwc_template));
pdev->vdev->parent = &intf->dev; pdev->vdev->parent = &intf->dev;
...@@ -1806,25 +1800,23 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1806,25 +1800,23 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
} }
pdev->vdev->release = video_device_release; pdev->vdev->release = video_device_release;
i = video_register_device(pdev->vdev, VFL_TYPE_GRABBER, video_nr); rc = video_register_device(pdev->vdev, VFL_TYPE_GRABBER, video_nr);
if (i < 0) { if (rc < 0) {
PWC_ERROR("Failed to register as video device (%d).\n", i); PWC_ERROR("Failed to register as video device (%d).\n", rc);
rc = i; goto err_video_release;
goto err;
} }
else {
PWC_INFO("Registered as /dev/video%d.\n", pdev->vdev->num); PWC_INFO("Registered as /dev/video%d.\n", pdev->vdev->num);
}
/* occupy slot */ /* occupy slot */
if (hint < MAX_DEV_HINTS) if (hint < MAX_DEV_HINTS)
device_hint[hint].pdev = pdev; device_hint[hint].pdev = pdev;
PWC_DEBUG_PROBE("probe() function returning struct at 0x%p.\n", pdev); PWC_DEBUG_PROBE("probe() function returning struct at 0x%p.\n", pdev);
usb_set_intfdata (intf, pdev); usb_set_intfdata(intf, pdev);
rc = pwc_create_sysfs_files(pdev->vdev); rc = pwc_create_sysfs_files(pdev->vdev);
if (rc) if (rc)
goto err_unreg; goto err_video_unreg;
/* Set the leds off */ /* Set the leds off */
pwc_set_leds(pdev, 0, 0); pwc_set_leds(pdev, 0, 0);
...@@ -1835,16 +1827,16 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1835,16 +1827,16 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
pdev->button_dev = input_allocate_device(); pdev->button_dev = input_allocate_device();
if (!pdev->button_dev) { if (!pdev->button_dev) {
PWC_ERROR("Err, insufficient memory for webcam snapshot button device."); PWC_ERROR("Err, insufficient memory for webcam snapshot button device.");
return -ENOMEM; rc = -ENOMEM;
pwc_remove_sysfs_files(pdev->vdev);
goto err_video_unreg;
} }
usb_make_path(udev, pdev->button_phys, sizeof(pdev->button_phys));
strlcat(pdev->button_phys, "/input0", sizeof(pdev->button_phys));
pdev->button_dev->name = "PWC snapshot button"; pdev->button_dev->name = "PWC snapshot button";
phys = kasprintf(GFP_KERNEL,"usb-%s-%s", pdev->udev->bus->bus_name, pdev->udev->devpath); pdev->button_dev->phys = pdev->button_phys;
if (!phys) {
input_free_device(pdev->button_dev);
return -ENOMEM;
}
pdev->button_dev->phys = phys;
usb_to_input_id(pdev->udev, &pdev->button_dev->id); usb_to_input_id(pdev->udev, &pdev->button_dev->id);
pdev->button_dev->dev.parent = &pdev->udev->dev; pdev->button_dev->dev.parent = &pdev->udev->dev;
pdev->button_dev->evbit[0] = BIT_MASK(EV_KEY); pdev->button_dev->evbit[0] = BIT_MASK(EV_KEY);
...@@ -1853,25 +1845,27 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id ...@@ -1853,25 +1845,27 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
rc = input_register_device(pdev->button_dev); rc = input_register_device(pdev->button_dev);
if (rc) { if (rc) {
input_free_device(pdev->button_dev); input_free_device(pdev->button_dev);
kfree(pdev->button_dev->phys);
pdev->button_dev = NULL; pdev->button_dev = NULL;
return rc; pwc_remove_sysfs_files(pdev->vdev);
goto err_video_unreg;
} }
#endif #endif
return 0; return 0;
err_unreg: err_video_unreg:
if (hint < MAX_DEV_HINTS) if (hint < MAX_DEV_HINTS)
device_hint[hint].pdev = NULL; device_hint[hint].pdev = NULL;
video_unregister_device(pdev->vdev); video_unregister_device(pdev->vdev);
err: pdev->vdev = NULL; /* So we don't try to release it below */
video_device_release(pdev->vdev); /* Drip... drip... drip... */ err_video_release:
kfree(pdev); /* Oops, no memory leaks please */ video_device_release(pdev->vdev);
err_free_mem:
kfree(pdev);
return rc; return rc;
} }
/* The user janked out the cable... */ /* The user yanked out the cable... */
static void usb_pwc_disconnect(struct usb_interface *intf) static void usb_pwc_disconnect(struct usb_interface *intf)
{ {
struct pwc_device *pdev; struct pwc_device *pdev;
...@@ -1902,7 +1896,7 @@ static void usb_pwc_disconnect(struct usb_interface *intf) ...@@ -1902,7 +1896,7 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
/* Alert waiting processes */ /* Alert waiting processes */
wake_up_interruptible(&pdev->frameq); wake_up_interruptible(&pdev->frameq);
/* Wait until device is closed */ /* Wait until device is closed */
if(pdev->vopen) { if (pdev->vopen) {
mutex_lock(&pdev->modlock); mutex_lock(&pdev->modlock);
pdev->unplugged = 1; pdev->unplugged = 1;
mutex_unlock(&pdev->modlock); mutex_unlock(&pdev->modlock);
...@@ -1911,8 +1905,6 @@ static void usb_pwc_disconnect(struct usb_interface *intf) ...@@ -1911,8 +1905,6 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
/* Device is closed, so we can safely unregister it */ /* Device is closed, so we can safely unregister it */
PWC_DEBUG_PROBE("Unregistering video device in disconnect().\n"); PWC_DEBUG_PROBE("Unregistering video device in disconnect().\n");
pwc_cleanup(pdev); pwc_cleanup(pdev);
/* Free memory (don't set pdev to 0 just yet) */
kfree(pdev);
disconnect_out: disconnect_out:
/* search device_hint[] table if we occupy a slot, by any chance */ /* search device_hint[] table if we occupy a slot, by any chance */
......
...@@ -253,6 +253,7 @@ struct pwc_device ...@@ -253,6 +253,7 @@ struct pwc_device
int snapshot_button_status; /* set to 1 when the user push the button, reset to 0 when this value is read */ int snapshot_button_status; /* set to 1 when the user push the button, reset to 0 when this value is read */
#ifdef CONFIG_USB_PWC_INPUT_EVDEV #ifdef CONFIG_USB_PWC_INPUT_EVDEV
struct input_dev *button_dev; /* webcam snapshot button input */ struct input_dev *button_dev; /* webcam snapshot button input */
char button_phys[64];
#endif #endif
/*** Misc. data ***/ /*** Misc. data ***/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment