Commit 254902b0 authored by Hans Verkuil's avatar Hans Verkuil Committed by Mauro Carvalho Chehab

[media] gspca: Fix locking issues related to suspend/resume

There are two bugs here: first the calls to stop0 (in gspca_suspend) and
gspca_init_transfer (in gspca_resume) need to be called with the usb_lock held.
That's true for the other places they are called and it is what subdrivers
expect. Quite a few will unlock the usb_lock in stop0 while waiting for a
worker thread to finish, and if usb_lock isn't held then that can cause a
kernel oops.

The other problem is that a worker thread needs to detect that it has to
halt due to a suspend. Otherwise it will just go on looping. So add tests
against gspca_dev->frozen in the worker threads that need it.

Hdg, 2 minor changes:
1) The finepix device is ok with stopping reading a frame halfway through,
   so add frozen checks in all places where we also check if we're still
   streaming
2) Use gspca_dev->dev instead of gspca_dev->present to check for disconnect
   in all touched drivers. I plan to do this everywhere in the future, and
   most relevant lines in the touched drivers are already modified by this
   patch.
Signed-off-by: default avatarHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent cc7b6f25
...@@ -94,7 +94,7 @@ static void dostream(struct work_struct *work) ...@@ -94,7 +94,7 @@ static void dostream(struct work_struct *work)
/* loop reading a frame */ /* loop reading a frame */
again: again:
while (gspca_dev->present && gspca_dev->streaming) { while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
/* request a frame */ /* request a frame */
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
...@@ -102,7 +102,8 @@ static void dostream(struct work_struct *work) ...@@ -102,7 +102,8 @@ static void dostream(struct work_struct *work)
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);
if (ret < 0) if (ret < 0)
break; break;
if (!gspca_dev->present || !gspca_dev->streaming) if (gspca_dev->frozen || !gspca_dev->dev ||
!gspca_dev->streaming)
break; break;
/* the frame comes in parts */ /* the frame comes in parts */
...@@ -117,7 +118,8 @@ static void dostream(struct work_struct *work) ...@@ -117,7 +118,8 @@ static void dostream(struct work_struct *work)
* error. Just restart. */ * error. Just restart. */
goto again; goto again;
} }
if (!gspca_dev->present || !gspca_dev->streaming) if (gspca_dev->frozen || !gspca_dev->dev ||
!gspca_dev->streaming)
goto out; goto out;
if (len < FPIX_MAX_TRANSFER || if (len < FPIX_MAX_TRANSFER ||
(data[len - 2] == 0xff && (data[len - 2] == 0xff &&
......
...@@ -2499,8 +2499,11 @@ int gspca_suspend(struct usb_interface *intf, pm_message_t message) ...@@ -2499,8 +2499,11 @@ int gspca_suspend(struct usb_interface *intf, pm_message_t message)
destroy_urbs(gspca_dev); destroy_urbs(gspca_dev);
gspca_input_destroy_urb(gspca_dev); gspca_input_destroy_urb(gspca_dev);
gspca_set_alt0(gspca_dev); gspca_set_alt0(gspca_dev);
if (gspca_dev->sd_desc->stop0) if (gspca_dev->sd_desc->stop0) {
mutex_lock(&gspca_dev->usb_lock);
gspca_dev->sd_desc->stop0(gspca_dev); gspca_dev->sd_desc->stop0(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock);
}
return 0; return 0;
} }
EXPORT_SYMBOL(gspca_suspend); EXPORT_SYMBOL(gspca_suspend);
...@@ -2508,7 +2511,7 @@ EXPORT_SYMBOL(gspca_suspend); ...@@ -2508,7 +2511,7 @@ EXPORT_SYMBOL(gspca_suspend);
int gspca_resume(struct usb_interface *intf) int gspca_resume(struct usb_interface *intf)
{ {
struct gspca_dev *gspca_dev = usb_get_intfdata(intf); struct gspca_dev *gspca_dev = usb_get_intfdata(intf);
int streaming; int streaming, ret = 0;
gspca_dev->frozen = 0; gspca_dev->frozen = 0;
gspca_dev->sd_desc->init(gspca_dev); gspca_dev->sd_desc->init(gspca_dev);
...@@ -2521,9 +2524,12 @@ int gspca_resume(struct usb_interface *intf) ...@@ -2521,9 +2524,12 @@ int gspca_resume(struct usb_interface *intf)
streaming = gspca_dev->streaming; streaming = gspca_dev->streaming;
gspca_dev->streaming = 0; gspca_dev->streaming = 0;
v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler); v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
if (streaming) if (streaming) {
return gspca_init_transfer(gspca_dev); mutex_lock(&gspca_dev->queue_lock);
return 0; ret = gspca_init_transfer(gspca_dev);
mutex_unlock(&gspca_dev->queue_lock);
}
return ret;
} }
EXPORT_SYMBOL(gspca_resume); EXPORT_SYMBOL(gspca_resume);
#endif #endif
......
...@@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work) ...@@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work)
goto quit_stream; goto quit_stream;
} }
while (gspca_dev->present && gspca_dev->streaming) { while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
/* Check if this is a new frame. If so, start the frame first */ /* Check if this is a new frame. If so, start the frame first */
if (!header_read) { if (!header_read) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
...@@ -367,7 +367,7 @@ static void jl2005c_dostream(struct work_struct *work) ...@@ -367,7 +367,7 @@ static void jl2005c_dostream(struct work_struct *work)
buffer, act_len); buffer, act_len);
header_read = 1; header_read = 1;
} }
while (bytes_left > 0 && gspca_dev->present) { while (bytes_left > 0 && gspca_dev->dev) {
data_len = bytes_left > JL2005C_MAX_TRANSFER ? data_len = bytes_left > JL2005C_MAX_TRANSFER ?
JL2005C_MAX_TRANSFER : bytes_left; JL2005C_MAX_TRANSFER : bytes_left;
ret = usb_bulk_msg(gspca_dev->dev, ret = usb_bulk_msg(gspca_dev->dev,
...@@ -390,7 +390,7 @@ static void jl2005c_dostream(struct work_struct *work) ...@@ -390,7 +390,7 @@ static void jl2005c_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->present) { if (gspca_dev->dev) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
jl2005c_stop(gspca_dev); jl2005c_stop(gspca_dev);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);
......
...@@ -232,7 +232,7 @@ static void sq905_dostream(struct work_struct *work) ...@@ -232,7 +232,7 @@ static void sq905_dostream(struct work_struct *work)
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage
+ FRAME_HEADER_LEN; + FRAME_HEADER_LEN;
while (gspca_dev->present && gspca_dev->streaming) { while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
/* request some data and then read it until we have /* request some data and then read it until we have
* a complete frame. */ * a complete frame. */
bytes_left = frame_sz; bytes_left = frame_sz;
...@@ -242,7 +242,7 @@ static void sq905_dostream(struct work_struct *work) ...@@ -242,7 +242,7 @@ static void sq905_dostream(struct work_struct *work)
we must finish reading an entire frame, otherwise the we must finish reading an entire frame, otherwise the
next time we stream we start reading in the middle of a next time we stream we start reading in the middle of a
frame. */ frame. */
while (bytes_left > 0 && gspca_dev->present) { while (bytes_left > 0 && gspca_dev->dev) {
data_len = bytes_left > SQ905_MAX_TRANSFER ? data_len = bytes_left > SQ905_MAX_TRANSFER ?
SQ905_MAX_TRANSFER : bytes_left; SQ905_MAX_TRANSFER : bytes_left;
ret = sq905_read_data(gspca_dev, buffer, data_len, 1); ret = sq905_read_data(gspca_dev, buffer, data_len, 1);
...@@ -274,7 +274,7 @@ static void sq905_dostream(struct work_struct *work) ...@@ -274,7 +274,7 @@ static void sq905_dostream(struct work_struct *work)
gspca_frame_add(gspca_dev, LAST_PACKET, gspca_frame_add(gspca_dev, LAST_PACKET,
NULL, 0); NULL, 0);
} }
if (gspca_dev->present) { if (gspca_dev->dev) {
/* acknowledge the frame */ /* acknowledge the frame */
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
ret = sq905_ack_frame(gspca_dev); ret = sq905_ack_frame(gspca_dev);
...@@ -284,7 +284,7 @@ static void sq905_dostream(struct work_struct *work) ...@@ -284,7 +284,7 @@ static void sq905_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->present) { if (gspca_dev->dev) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sq905_command(gspca_dev, SQ905_CLEAR); sq905_command(gspca_dev, SQ905_CLEAR);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);
......
...@@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work) ...@@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work)
goto quit_stream; goto quit_stream;
} }
while (gspca_dev->present && gspca_dev->streaming) { while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
/* Request the header, which tells the size to download */ /* Request the header, which tells the size to download */
ret = usb_bulk_msg(gspca_dev->dev, ret = usb_bulk_msg(gspca_dev->dev,
usb_rcvbulkpipe(gspca_dev->dev, 0x81), usb_rcvbulkpipe(gspca_dev->dev, 0x81),
...@@ -169,7 +169,7 @@ static void sq905c_dostream(struct work_struct *work) ...@@ -169,7 +169,7 @@ static void sq905c_dostream(struct work_struct *work)
packet_type = FIRST_PACKET; packet_type = FIRST_PACKET;
gspca_frame_add(gspca_dev, packet_type, gspca_frame_add(gspca_dev, packet_type,
buffer, FRAME_HEADER_LEN); buffer, FRAME_HEADER_LEN);
while (bytes_left > 0 && gspca_dev->present) { while (bytes_left > 0 && gspca_dev->dev) {
data_len = bytes_left > SQ905C_MAX_TRANSFER ? data_len = bytes_left > SQ905C_MAX_TRANSFER ?
SQ905C_MAX_TRANSFER : bytes_left; SQ905C_MAX_TRANSFER : bytes_left;
ret = usb_bulk_msg(gspca_dev->dev, ret = usb_bulk_msg(gspca_dev->dev,
...@@ -191,7 +191,7 @@ static void sq905c_dostream(struct work_struct *work) ...@@ -191,7 +191,7 @@ static void sq905c_dostream(struct work_struct *work)
} }
} }
quit_stream: quit_stream:
if (gspca_dev->present) { if (gspca_dev->dev) {
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sq905c_command(gspca_dev, SQ905C_CLEAR, 0); sq905c_command(gspca_dev, SQ905C_CLEAR, 0);
mutex_unlock(&gspca_dev->usb_lock); mutex_unlock(&gspca_dev->usb_lock);
......
...@@ -225,7 +225,7 @@ static void vicam_dostream(struct work_struct *work) ...@@ -225,7 +225,7 @@ static void vicam_dostream(struct work_struct *work)
goto exit; goto exit;
} }
while (gspca_dev->present && gspca_dev->streaming) { while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
ret = vicam_read_frame(gspca_dev, buffer, frame_sz); ret = vicam_read_frame(gspca_dev, buffer, frame_sz);
if (ret < 0) if (ret < 0)
break; break;
...@@ -327,7 +327,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) ...@@ -327,7 +327,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
dev->work_thread = NULL; dev->work_thread = NULL;
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
if (gspca_dev->present) if (gspca_dev->dev)
vicam_set_camera_power(gspca_dev, 0); vicam_set_camera_power(gspca_dev, 0);
} }
......
...@@ -6093,7 +6093,8 @@ static void transfer_update(struct work_struct *work) ...@@ -6093,7 +6093,8 @@ static void transfer_update(struct work_struct *work)
/* get the transfer status */ /* get the transfer status */
/* the bit 0 of the bridge register 11 indicates overflow */ /* the bit 0 of the bridge register 11 indicates overflow */
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
if (!gspca_dev->present || !gspca_dev->streaming) if (gspca_dev->frozen || !gspca_dev->dev ||
!gspca_dev->streaming)
goto err; goto err;
reg11 = reg_r(gspca_dev, 0x0011); reg11 = reg_r(gspca_dev, 0x0011);
if (gspca_dev->usb_err < 0 if (gspca_dev->usb_err < 0
...@@ -6949,7 +6950,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev) ...@@ -6949,7 +6950,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
mutex_lock(&gspca_dev->usb_lock); mutex_lock(&gspca_dev->usb_lock);
sd->work_thread = NULL; sd->work_thread = NULL;
} }
if (!gspca_dev->present) if (!gspca_dev->dev)
return; return;
send_unknown(gspca_dev, sd->sensor); send_unknown(gspca_dev, sd->sensor);
} }
......
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