Commit 7536e0dce329aabda3f06a0356ac974bcdb42cae
Committed by
Greg Kroah-Hartman
1 parent
5d9aa9e5e8
vb2: fix vb2_thread_stop race conditions
commit 6cf11ee6300f38b7cfc43af9b7be2afaa5e05869 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: Hans Verkuil <hans.verkuil@cisco.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Showing 1 changed file with 9 additions and 10 deletions Side-by-side Diff
drivers/media/v4l2-core/videobuf2-core.c
... | ... | @@ -3142,27 +3142,26 @@ |
3142 | 3142 | prequeue--; |
3143 | 3143 | } else { |
3144 | 3144 | call_void_qop(q, wait_finish, q); |
3145 | - ret = vb2_internal_dqbuf(q, &fileio->b, 0); | |
3145 | + if (!threadio->stop) | |
3146 | + ret = vb2_internal_dqbuf(q, &fileio->b, 0); | |
3146 | 3147 | call_void_qop(q, wait_prepare, q); |
3147 | 3148 | dprintk(5, "file io: vb2_dqbuf result: %d\n", ret); |
3148 | 3149 | } |
3149 | - if (threadio->stop) | |
3150 | + if (ret || threadio->stop) | |
3150 | 3151 | break; |
3151 | - if (ret) | |
3152 | - break; | |
3153 | 3152 | try_to_freeze(); |
3154 | 3153 | |
3155 | 3154 | vb = q->bufs[fileio->b.index]; |
3156 | 3155 | if (!(fileio->b.flags & V4L2_BUF_FLAG_ERROR)) |
3157 | - ret = threadio->fnc(vb, threadio->priv); | |
3158 | - if (ret) | |
3159 | - break; | |
3156 | + if (threadio->fnc(vb, threadio->priv)) | |
3157 | + break; | |
3160 | 3158 | call_void_qop(q, wait_finish, q); |
3161 | 3159 | if (set_timestamp) |
3162 | 3160 | v4l2_get_timestamp(&fileio->b.timestamp); |
3163 | - ret = vb2_internal_qbuf(q, &fileio->b); | |
3161 | + if (!threadio->stop) | |
3162 | + ret = vb2_internal_qbuf(q, &fileio->b); | |
3164 | 3163 | call_void_qop(q, wait_prepare, q); |
3165 | - if (ret) | |
3164 | + if (ret || threadio->stop) | |
3166 | 3165 | break; |
3167 | 3166 | } |
3168 | 3167 | |
3169 | 3168 | |
... | ... | @@ -3231,11 +3230,11 @@ |
3231 | 3230 | threadio->stop = true; |
3232 | 3231 | vb2_internal_streamoff(q, q->type); |
3233 | 3232 | call_void_qop(q, wait_prepare, q); |
3233 | + err = kthread_stop(threadio->thread); | |
3234 | 3234 | q->fileio = NULL; |
3235 | 3235 | fileio->req.count = 0; |
3236 | 3236 | vb2_reqbufs(q, &fileio->req); |
3237 | 3237 | kfree(fileio); |
3238 | - err = kthread_stop(threadio->thread); | |
3239 | 3238 | threadio->thread = NULL; |
3240 | 3239 | kfree(threadio); |
3241 | 3240 | q->fileio = NULL; |