lkml.org 
[lkml]   [2019]   [Nov]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 284/306] media: vivid: Fix wrong locking that causes race conditions on streaming stop
    Date
    From: Alexander Popov <alex.popov@linux.com>

    commit 6dcd5d7a7a29c1e4b8016a06aed78cd650cd8c27 upstream.

    There is the same incorrect approach to locking implemented in
    vivid_stop_generating_vid_cap(), vivid_stop_generating_vid_out() and
    sdr_cap_stop_streaming().

    These functions are called during streaming stopping with vivid_dev.mutex
    locked. And they all do the same mistake while stopping their kthreads,
    which need to lock this mutex as well. See the example from
    vivid_stop_generating_vid_cap():
    /* shutdown control thread */
    vivid_grab_controls(dev, false);
    mutex_unlock(&dev->mutex);
    kthread_stop(dev->kthread_vid_cap);
    dev->kthread_vid_cap = NULL;
    mutex_lock(&dev->mutex);

    But when this mutex is unlocked, another vb2_fop_read() can lock it
    instead of vivid_thread_vid_cap() and manipulate the buffer queue.
    That causes a use-after-free access later.

    To fix those issues let's:
    1. avoid unlocking the mutex in vivid_stop_generating_vid_cap(),
    vivid_stop_generating_vid_out() and sdr_cap_stop_streaming();
    2. use mutex_trylock() with schedule_timeout_uninterruptible() in
    the loops of the vivid kthread handlers.

    Signed-off-by: Alexander Popov <alex.popov@linux.com>
    Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
    Tested-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
    Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
    Cc: <stable@vger.kernel.org> # for v3.18 and up
    Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/media/platform/vivid/vivid-kthread-cap.c | 8 +++++---
    drivers/media/platform/vivid/vivid-kthread-out.c | 8 +++++---
    drivers/media/platform/vivid/vivid-sdr-cap.c | 8 +++++---
    3 files changed, 15 insertions(+), 9 deletions(-)

    --- a/drivers/media/platform/vivid/vivid-kthread-cap.c
    +++ b/drivers/media/platform/vivid/vivid-kthread-cap.c
    @@ -765,7 +765,11 @@ static int vivid_thread_vid_cap(void *da
    if (kthread_should_stop())
    break;

    - mutex_lock(&dev->mutex);
    + if (!mutex_trylock(&dev->mutex)) {
    + schedule_timeout_uninterruptible(1);
    + continue;
    + }
    +
    cur_jiffies = jiffies;
    if (dev->cap_seq_resync) {
    dev->jiffies_vid_cap = cur_jiffies;
    @@ -918,8 +922,6 @@ void vivid_stop_generating_vid_cap(struc

    /* shutdown control thread */
    vivid_grab_controls(dev, false);
    - mutex_unlock(&dev->mutex);
    kthread_stop(dev->kthread_vid_cap);
    dev->kthread_vid_cap = NULL;
    - mutex_lock(&dev->mutex);
    }
    --- a/drivers/media/platform/vivid/vivid-kthread-out.c
    +++ b/drivers/media/platform/vivid/vivid-kthread-out.c
    @@ -135,7 +135,11 @@ static int vivid_thread_vid_out(void *da
    if (kthread_should_stop())
    break;

    - mutex_lock(&dev->mutex);
    + if (!mutex_trylock(&dev->mutex)) {
    + schedule_timeout_uninterruptible(1);
    + continue;
    + }
    +
    cur_jiffies = jiffies;
    if (dev->out_seq_resync) {
    dev->jiffies_vid_out = cur_jiffies;
    @@ -289,8 +293,6 @@ void vivid_stop_generating_vid_out(struc

    /* shutdown control thread */
    vivid_grab_controls(dev, false);
    - mutex_unlock(&dev->mutex);
    kthread_stop(dev->kthread_vid_out);
    dev->kthread_vid_out = NULL;
    - mutex_lock(&dev->mutex);
    }
    --- a/drivers/media/platform/vivid/vivid-sdr-cap.c
    +++ b/drivers/media/platform/vivid/vivid-sdr-cap.c
    @@ -137,7 +137,11 @@ static int vivid_thread_sdr_cap(void *da
    if (kthread_should_stop())
    break;

    - mutex_lock(&dev->mutex);
    + if (!mutex_trylock(&dev->mutex)) {
    + schedule_timeout_uninterruptible(1);
    + continue;
    + }
    +
    cur_jiffies = jiffies;
    if (dev->sdr_cap_seq_resync) {
    dev->jiffies_sdr_cap = cur_jiffies;
    @@ -297,10 +301,8 @@ static void sdr_cap_stop_streaming(struc
    }

    /* shutdown control thread */
    - mutex_unlock(&dev->mutex);
    kthread_stop(dev->kthread_sdr_cap);
    dev->kthread_sdr_cap = NULL;
    - mutex_lock(&dev->mutex);
    }

    const struct vb2_ops vivid_sdr_cap_qops = {

    \
     
     \ /
      Last update: 2019-11-27 22:08    [W:4.119 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site