From 3a0efc3200386b9288e1d3d3be0a9f5d6f286906 Mon Sep 17 00:00:00 2001 From: Alexey Klimov Date: Sat, 27 Dec 2008 21:32:49 -0300 Subject: [PATCH] V4L/DVB (10054): dsbr100: fix unplug oops This patch corrects unplug procedure. Patch adds usb_dsbr100_video_device_release, new macros - videodev_to_radio, mutex lock and a lot of safety checks. Struct video_device videodev is embedded in dsbr100_device structure. Signed-off-by: Alexey Klimov Signed-off-by: Douglas Schilling Landgraf Signed-off-by: Mauro Carvalho Chehab --- drivers/media/radio/dsbr100.c | 106 ++++++++++++++++++++++++---------- 1 file changed, 77 insertions(+), 29 deletions(-) diff --git a/drivers/media/radio/dsbr100.c b/drivers/media/radio/dsbr100.c index eafa547ca96..84914fb267b 100644 --- a/drivers/media/radio/dsbr100.c +++ b/drivers/media/radio/dsbr100.c @@ -145,6 +145,7 @@ devices, that would be 76 and 91. */ #define FREQ_MAX 108.0 #define FREQ_MUL 16000 +#define videodev_to_radio(d) container_of(d, struct dsbr100_device, videodev) static int usb_dsbr100_probe(struct usb_interface *intf, const struct usb_device_id *id); @@ -161,8 +162,9 @@ module_param(radio_nr, int, 0); /* Data for one (physical) device */ struct dsbr100_device { struct usb_device *usbdev; - struct video_device *videodev; + struct video_device videodev; u8 *transfer_buffer; + struct mutex lock; /* buffer locking */ int curfreq; int stereo; int users; @@ -195,6 +197,7 @@ static struct usb_driver usb_dsbr100_driver = { /* switch on radio */ static int dsbr100_start(struct dsbr100_device *radio) { + mutex_lock(&radio->lock); if (usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), USB_REQ_GET_STATUS, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, @@ -202,9 +205,13 @@ static int dsbr100_start(struct dsbr100_device *radio) usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), DSB100_ONOFF, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x01, 0x00, radio->transfer_buffer, 8, 300) < 0) + 0x01, 0x00, radio->transfer_buffer, 8, 300) < 0) { + mutex_unlock(&radio->lock); return -1; + } + radio->muted=0; + mutex_unlock(&radio->lock); return (radio->transfer_buffer)[0]; } @@ -212,6 +219,7 @@ static int dsbr100_start(struct dsbr100_device *radio) /* switch off radio */ static int dsbr100_stop(struct dsbr100_device *radio) { + mutex_lock(&radio->lock); if (usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), USB_REQ_GET_STATUS, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, @@ -219,9 +227,13 @@ static int dsbr100_stop(struct dsbr100_device *radio) usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), DSB100_ONOFF, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x00, 0x00, radio->transfer_buffer, 8, 300) < 0) + 0x00, 0x00, radio->transfer_buffer, 8, 300) < 0) { + mutex_unlock(&radio->lock); return -1; + } + radio->muted=1; + mutex_unlock(&radio->lock); return (radio->transfer_buffer)[0]; } @@ -229,6 +241,7 @@ static int dsbr100_stop(struct dsbr100_device *radio) static int dsbr100_setfreq(struct dsbr100_device *radio, int freq) { freq = (freq / 16 * 80) / 1000 + 856; + mutex_lock(&radio->lock); if (usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), DSB100_TUNE, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, @@ -243,9 +256,12 @@ static int dsbr100_setfreq(struct dsbr100_device *radio, int freq) USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, 0x00, 0x24, radio->transfer_buffer, 8, 300) < 0) { radio->stereo = -1; + mutex_unlock(&radio->lock); return -1; } + radio->stereo = !((radio->transfer_buffer)[0] & 0x01); + mutex_unlock(&radio->lock); return (radio->transfer_buffer)[0]; } @@ -253,6 +269,7 @@ static int dsbr100_setfreq(struct dsbr100_device *radio, int freq) sees a stereo signal or not. Pity. */ static void dsbr100_getstat(struct dsbr100_device *radio) { + mutex_lock(&radio->lock); if (usb_control_msg(radio->usbdev, usb_rcvctrlpipe(radio->usbdev, 0), USB_REQ_GET_STATUS, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, @@ -260,6 +277,7 @@ static void dsbr100_getstat(struct dsbr100_device *radio) radio->stereo = -1; else radio->stereo = !(radio->transfer_buffer[0] & 0x01); + mutex_unlock(&radio->lock); } @@ -274,16 +292,12 @@ static void usb_dsbr100_disconnect(struct usb_interface *intf) struct dsbr100_device *radio = usb_get_intfdata(intf); usb_set_intfdata (intf, NULL); - if (radio) { - video_unregister_device(radio->videodev); - radio->videodev = NULL; - if (radio->users) { - kfree(radio->transfer_buffer); - kfree(radio); - } else { - radio->removed = 1; - } - } + + mutex_lock(&radio->lock); + radio->removed = 1; + mutex_unlock(&radio->lock); + + video_unregister_device(&radio->videodev); } @@ -303,6 +317,10 @@ static int vidioc_g_tuner(struct file *file, void *priv, { struct dsbr100_device *radio = video_drvdata(file); + /* safety check */ + if (radio->removed) + return -EIO; + if (v->index > 0) return -EINVAL; @@ -324,6 +342,12 @@ static int vidioc_g_tuner(struct file *file, void *priv, static int vidioc_s_tuner(struct file *file, void *priv, struct v4l2_tuner *v) { + struct dsbr100_device *radio = video_drvdata(file); + + /* safety check */ + if (radio->removed) + return -EIO; + if (v->index > 0) return -EINVAL; @@ -335,6 +359,10 @@ static int vidioc_s_frequency(struct file *file, void *priv, { struct dsbr100_device *radio = video_drvdata(file); + /* safety check */ + if (radio->removed) + return -EIO; + radio->curfreq = f->frequency; if (dsbr100_setfreq(radio, radio->curfreq) == -1) dev_warn(&radio->usbdev->dev, "Set frequency failed\n"); @@ -346,6 +374,10 @@ static int vidioc_g_frequency(struct file *file, void *priv, { struct dsbr100_device *radio = video_drvdata(file); + /* safety check */ + if (radio->removed) + return -EIO; + f->type = V4L2_TUNER_RADIO; f->frequency = radio->curfreq; return 0; @@ -370,6 +402,10 @@ static int vidioc_g_ctrl(struct file *file, void *priv, { struct dsbr100_device *radio = video_drvdata(file); + /* safety check */ + if (radio->removed) + return -EIO; + switch (ctrl->id) { case V4L2_CID_AUDIO_MUTE: ctrl->value = radio->muted; @@ -383,6 +419,10 @@ static int vidioc_s_ctrl(struct file *file, void *priv, { struct dsbr100_device *radio = video_drvdata(file); + /* safety check */ + if (radio->removed) + return -EIO; + switch (ctrl->id) { case V4L2_CID_AUDIO_MUTE: if (ctrl->value) { @@ -464,13 +504,19 @@ static int usb_dsbr100_open(struct inode *inode, struct file *file) static int usb_dsbr100_close(struct inode *inode, struct file *file) { struct dsbr100_device *radio = video_drvdata(file); + int retval; if (!radio) return -ENODEV; + radio->users = 0; - if (radio->removed) { - kfree(radio->transfer_buffer); - kfree(radio); + if (!radio->removed) { + retval = dsbr100_stop(radio); + if (retval == -1) { + dev_warn(&radio->usbdev->dev, + "dsbr100_stop failed\n"); + } + } return 0; } @@ -505,6 +551,14 @@ static int usb_dsbr100_resume(struct usb_interface *intf) return 0; } +static void usb_dsbr100_video_device_release(struct video_device *videodev) +{ + struct dsbr100_device *radio = videodev_to_radio(videodev); + + kfree(radio->transfer_buffer); + kfree(radio); +} + /* File system interface */ static const struct file_operations usb_dsbr100_fops = { .owner = THIS_MODULE, @@ -533,11 +587,11 @@ static const struct v4l2_ioctl_ops usb_dsbr100_ioctl_ops = { }; /* V4L2 interface */ -static struct video_device dsbr100_videodev_template = { +static struct video_device dsbr100_videodev_data = { .name = "D-Link DSB-R 100", .fops = &usb_dsbr100_fops, .ioctl_ops = &usb_dsbr100_ioctl_ops, - .release = video_device_release, + .release = usb_dsbr100_video_device_release, }; /* check if the device is present and register with v4l and @@ -558,23 +612,17 @@ static int usb_dsbr100_probe(struct usb_interface *intf, kfree(radio); return -ENOMEM; } - radio->videodev = video_device_alloc(); - if (!(radio->videodev)) { - kfree(radio->transfer_buffer); - kfree(radio); - return -ENOMEM; - } - memcpy(radio->videodev, &dsbr100_videodev_template, - sizeof(dsbr100_videodev_template)); + mutex_init(&radio->lock); + radio->videodev = dsbr100_videodev_data; + radio->removed = 0; radio->users = 0; radio->usbdev = interface_to_usbdev(intf); radio->curfreq = FREQ_MIN * FREQ_MUL; - video_set_drvdata(radio->videodev, radio); - if (video_register_device(radio->videodev, VFL_TYPE_RADIO, radio_nr) < 0) { + video_set_drvdata(&radio->videodev, radio); + if (video_register_device(&radio->videodev, VFL_TYPE_RADIO, radio_nr) < 0) { dev_warn(&intf->dev, "Could not register video device\n"); - video_device_release(radio->videodev); kfree(radio->transfer_buffer); kfree(radio); return -EIO;