[media] vb2: fix vb2_thread_stop race conditions

The locking scheme inside the vb2 thread is unsafe when stopping the
thread. In particular kthread_stop was called *after* internal data
structures were cleaned up instead of doing that before. In addition,
internal vb2 functions were called after threadio->stop was set to
true and vb2_internal_streamoff was called. This is also not allowed.

All this led to a variety of race conditions and kernel warnings and/or
oopses.

Fixed by moving the kthread_stop call up before the cleanup takes
place, and by checking threadio->stop before calling internal vb2
queuing operations.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: <stable@vger.kernel.org>      # for v3.16 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

1 files changed, 9 insertions, 10 deletions

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index d09a8916e940..bc08a829bc13 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -3146,27 +3146,26 @@ static int vb2_thread(void *data)
                         prequeue--;
                 } else {
                         call_void_qop(q, wait_finish, q);
-                        ret = vb2_internal_dqbuf(q, &fileio->b, 0);
+                        if (!threadio->stop)
+                                ret = vb2_internal_dqbuf(q, &fileio->b, 0);
                         call_void_qop(q, wait_prepare, q);
                         dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
                 }
-                if (threadio->stop)
-                        break;
-                if (ret)
+                if (ret || threadio->stop)
                         break;
                 try_to_freeze();
 
                 vb = q->bufs[fileio->b.index];
                 if (!(fileio->b.flags & V4L2_BUF_FLAG_ERROR))
-                        ret = threadio->fnc(vb, threadio->priv);
-                if (ret)
-                        break;
+                        if (threadio->fnc(vb, threadio->priv))
+                                break;
                 call_void_qop(q, wait_finish, q);
                 if (set_timestamp)
                         v4l2_get_timestamp(&fileio->b.timestamp);
-                ret = vb2_internal_qbuf(q, &fileio->b);
+                if (!threadio->stop)
+                        ret = vb2_internal_qbuf(q, &fileio->b);
                 call_void_qop(q, wait_prepare, q);
-                if (ret)
+                if (ret || threadio->stop)
                         break;
         }
 
@@ -3235,11 +3234,11 @@ int vb2_thread_stop(struct vb2_queue *q)
         threadio->stop = true;
         vb2_internal_streamoff(q, q->type);
         call_void_qop(q, wait_prepare, q);
+        err = kthread_stop(threadio->thread);
         q->fileio = NULL;
         fileio->req.count = 0;
         vb2_reqbufs(q, &fileio->req);
         kfree(fileio);
-        err = kthread_stop(threadio->thread);
         threadio->thread = NULL;
         kfree(threadio);
         q->fileio = NULL;