Commit 67772bca authored by Hans Verkuil's avatar Hans Verkuil Committed by Luis Henriques

[media] vb2: fix vb2_thread_stop race conditions

commit 6cf11ee6 upstream.

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: default avatarHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 9c167b45
...@@ -3087,27 +3087,26 @@ static int vb2_thread(void *data) ...@@ -3087,27 +3087,26 @@ static int vb2_thread(void *data)
prequeue--; prequeue--;
} else { } else {
call_void_qop(q, wait_finish, q); 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); call_void_qop(q, wait_prepare, q);
dprintk(5, "file io: vb2_dqbuf result: %d\n", ret); dprintk(5, "file io: vb2_dqbuf result: %d\n", ret);
} }
if (threadio->stop) if (ret || threadio->stop)
break;
if (ret)
break; break;
try_to_freeze(); try_to_freeze();
vb = q->bufs[fileio->b.index]; vb = q->bufs[fileio->b.index];
if (!(fileio->b.flags & V4L2_BUF_FLAG_ERROR)) if (!(fileio->b.flags & V4L2_BUF_FLAG_ERROR))
ret = threadio->fnc(vb, threadio->priv); if (threadio->fnc(vb, threadio->priv))
if (ret) break;
break;
call_void_qop(q, wait_finish, q); call_void_qop(q, wait_finish, q);
if (set_timestamp) if (set_timestamp)
v4l2_get_timestamp(&fileio->b.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); call_void_qop(q, wait_prepare, q);
if (ret) if (ret || threadio->stop)
break; break;
} }
...@@ -3176,11 +3175,11 @@ int vb2_thread_stop(struct vb2_queue *q) ...@@ -3176,11 +3175,11 @@ int vb2_thread_stop(struct vb2_queue *q)
threadio->stop = true; threadio->stop = true;
vb2_internal_streamoff(q, q->type); vb2_internal_streamoff(q, q->type);
call_void_qop(q, wait_prepare, q); call_void_qop(q, wait_prepare, q);
err = kthread_stop(threadio->thread);
q->fileio = NULL; q->fileio = NULL;
fileio->req.count = 0; fileio->req.count = 0;
vb2_reqbufs(q, &fileio->req); vb2_reqbufs(q, &fileio->req);
kfree(fileio); kfree(fileio);
err = kthread_stop(threadio->thread);
threadio->thread = NULL; threadio->thread = NULL;
kfree(threadio); kfree(threadio);
q->fileio = NULL; q->fileio = NULL;
......
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