[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: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/gspca/finepix.c b/drivers/media/video/gspca/finepix.c
index 0107513cd728d5cafbb1dded25f051cc569a0217..d0befe981098a7ecf8f868fb61d3fbfcb157e11e 100644 (file)
--- a/drivers/media/video/gspca/finepix.c
+++ b/drivers/media/video/gspca/finepix.c
@@ -94,7 +94,7 @@ static void dostream(struct work_struct *work)
 
        /* loop reading a frame */
 again:
-       while (gspca_dev->present && gspca_dev->streaming) {
+       while (!gspca_dev->frozen && gspca_dev->dev && gspca_dev->streaming) {
 
                /* request a frame */
                mutex_lock(&gspca_dev->usb_lock);
@@ -102,7 +102,8 @@ again:
                mutex_unlock(&gspca_dev->usb_lock);
                if (ret < 0)
                        break;
-               if (!gspca_dev->present || !gspca_dev->streaming)
+               if (gspca_dev->frozen || !gspca_dev->dev ||
+                                        !gspca_dev->streaming)
                        break;
 
                /* the frame comes in parts */
@@ -117,7 +118,8 @@ again:
                                 * error. Just restart. */
                                goto again;
                        }
-                       if (!gspca_dev->present || !gspca_dev->streaming)
+                       if (gspca_dev->frozen || !gspca_dev->dev ||
+                                                !gspca_dev->streaming)
                                goto out;
                        if (len < FPIX_MAX_TRANSFER ||
                                (data[len - 2] == 0xff &&

diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c
index 7669f27238c33377fea624144f1854b79202cfcf..a14c8f71d48b1b8ec704ab359fb85560ef683adc 100644 (file)
--- a/drivers/media/video/gspca/gspca.c
+++ b/drivers/media/video/gspca/gspca.c
@@ -2499,8 +2499,11 @@ int gspca_suspend(struct usb_interface *intf, pm_message_t message)
        destroy_urbs(gspca_dev);
        gspca_input_destroy_urb(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);
+               mutex_unlock(&gspca_dev->usb_lock);
+       }
        return 0;
 }
 EXPORT_SYMBOL(gspca_suspend);
@@ -2508,7 +2511,7 @@ EXPORT_SYMBOL(gspca_suspend);
 int gspca_resume(struct usb_interface *intf)
 {
        struct gspca_dev *gspca_dev = usb_get_intfdata(intf);
-       int streaming;
+       int streaming, ret = 0;
 
        gspca_dev->frozen = 0;
        gspca_dev->sd_desc->init(gspca_dev);
@@ -2521,9 +2524,12 @@ int gspca_resume(struct usb_interface *intf)
        streaming = gspca_dev->streaming;
        gspca_dev->streaming = 0;
        v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
-       if (streaming)
-               return gspca_init_transfer(gspca_dev);
-       return 0;
+       if (streaming) {
+               mutex_lock(&gspca_dev->queue_lock);
+               ret = gspca_init_transfer(gspca_dev);
+               mutex_unlock(&gspca_dev->queue_lock);
+       }
+       return ret;
 }
 EXPORT_SYMBOL(gspca_resume);
 #endif

diff --git a/drivers/media/video/gspca/jl2005bcd.c b/drivers/media/video/gspca/jl2005bcd.c
index 53f58ef367cfa5bea36100d884e7225c07aa341b..e1fc2561e4bc9f6b3549f5d804910f3a20ca74d7 100644 (file)
--- a/drivers/media/video/gspca/jl2005bcd.c
+++ b/drivers/media/video/gspca/jl2005bcd.c
@@ -335,7 +335,7 @@ static void jl2005c_dostream(struct work_struct *work)
                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 */
                if (!header_read) {
                        mutex_lock(&gspca_dev->usb_lock);
@@ -367,7 +367,7 @@ static void jl2005c_dostream(struct work_struct *work)
                                        buffer, act_len);
                        header_read = 1;
                }
-               while (bytes_left > 0 && gspca_dev->present) {
+               while (bytes_left > 0 && gspca_dev->dev) {
                        data_len = bytes_left > JL2005C_MAX_TRANSFER ?
                                JL2005C_MAX_TRANSFER : bytes_left;
                        ret = usb_bulk_msg(gspca_dev->dev,
@@ -390,7 +390,7 @@ static void jl2005c_dostream(struct work_struct *work)
                }
        }
 quit_stream:
-       if (gspca_dev->present) {
+       if (gspca_dev->dev) {
                mutex_lock(&gspca_dev->usb_lock);
                jl2005c_stop(gspca_dev);
                mutex_unlock(&gspca_dev->usb_lock);

diff --git a/drivers/media/video/gspca/sq905.c b/drivers/media/video/gspca/sq905.c
index 2fe3c29bd6b79ca2f5707e9c78b65aad14f53d7f..a144ce759b662b4cf0bf4d5c1f0b99e59a22693b 100644 (file)
--- a/drivers/media/video/gspca/sq905.c
+++ b/drivers/media/video/gspca/sq905.c
@@ -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_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
                 * a complete frame. */
                bytes_left = frame_sz;
@@ -242,7 +242,7 @@ static void sq905_dostream(struct work_struct *work)
                   we must finish reading an entire frame, otherwise the
                   next time we stream we start reading in the middle of a
                   frame. */
-               while (bytes_left > 0 && gspca_dev->present) {
+               while (bytes_left > 0 && gspca_dev->dev) {
                        data_len = bytes_left > SQ905_MAX_TRANSFER ?
                                SQ905_MAX_TRANSFER : bytes_left;
                        ret = sq905_read_data(gspca_dev, buffer, data_len, 1);
@@ -274,7 +274,7 @@ static void sq905_dostream(struct work_struct *work)
                                gspca_frame_add(gspca_dev, LAST_PACKET,
                                                NULL, 0);
                }
-               if (gspca_dev->present) {
+               if (gspca_dev->dev) {
                        /* acknowledge the frame */
                        mutex_lock(&gspca_dev->usb_lock);
                        ret = sq905_ack_frame(gspca_dev);
@@ -284,7 +284,7 @@ static void sq905_dostream(struct work_struct *work)
                }
        }
 quit_stream:
-       if (gspca_dev->present) {
+       if (gspca_dev->dev) {
                mutex_lock(&gspca_dev->usb_lock);
                sq905_command(gspca_dev, SQ905_CLEAR);
                mutex_unlock(&gspca_dev->usb_lock);

diff --git a/drivers/media/video/gspca/sq905c.c b/drivers/media/video/gspca/sq905c.c
index ae783634712f4196a4751fe678d5370e65b5e2a0..720c187f6ec7af799fc9fd80b2bf99728cfd161c 100644 (file)
--- a/drivers/media/video/gspca/sq905c.c
+++ b/drivers/media/video/gspca/sq905c.c
@@ -150,7 +150,7 @@ static void sq905c_dostream(struct work_struct *work)
                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 */
                ret = usb_bulk_msg(gspca_dev->dev,
                                usb_rcvbulkpipe(gspca_dev->dev, 0x81),
@@ -169,7 +169,7 @@ static void sq905c_dostream(struct work_struct *work)
                packet_type = FIRST_PACKET;
                gspca_frame_add(gspca_dev, packet_type,
                                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 ?
                                SQ905C_MAX_TRANSFER : bytes_left;
                        ret = usb_bulk_msg(gspca_dev->dev,
@@ -191,7 +191,7 @@ static void sq905c_dostream(struct work_struct *work)
                }
        }
 quit_stream:
-       if (gspca_dev->present) {
+       if (gspca_dev->dev) {
                mutex_lock(&gspca_dev->usb_lock);
                sq905c_command(gspca_dev, SQ905C_CLEAR, 0);
                mutex_unlock(&gspca_dev->usb_lock);

diff --git a/drivers/media/video/gspca/vicam.c b/drivers/media/video/gspca/vicam.c
index e48ec4db6da4cbe881e0cc569768553d7fa68109..432d6cd99cd65915ea8f218bf77b4e08a3f37510 100644 (file)
--- a/drivers/media/video/gspca/vicam.c
+++ b/drivers/media/video/gspca/vicam.c
@@ -225,7 +225,7 @@ static void vicam_dostream(struct work_struct *work)
                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);
                if (ret < 0)
                        break;
@@ -327,7 +327,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
        dev->work_thread = NULL;
        mutex_lock(&gspca_dev->usb_lock);
 
-       if (gspca_dev->present)
+       if (gspca_dev->dev)
                vicam_set_camera_power(gspca_dev, 0);
 }
 

diff --git a/drivers/media/video/gspca/zc3xx.c b/drivers/media/video/gspca/zc3xx.c
index 7d9a4f1be9dced2432198421abb39a4bafb755b2..8f21bae46ef8e517e46ce596e34a5fc392df9577 100644 (file)
--- a/drivers/media/video/gspca/zc3xx.c
+++ b/drivers/media/video/gspca/zc3xx.c
@@ -6093,7 +6093,8 @@ static void transfer_update(struct work_struct *work)
                /* get the transfer status */
                /* the bit 0 of the bridge register 11 indicates overflow */
                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;
                reg11 = reg_r(gspca_dev, 0x0011);
                if (gspca_dev->usb_err < 0
@@ -6949,7 +6950,7 @@ static void sd_stop0(struct gspca_dev *gspca_dev)
                mutex_lock(&gspca_dev->usb_lock);
                sd->work_thread = NULL;
        }
-       if (!gspca_dev->present)
+       if (!gspca_dev->dev)
                return;
        send_unknown(gspca_dev, sd->sensor);
 }