diff options
| author | Pavan Kumar Chilamkurthi <pchilamk@codeaurora.org> | 2016-12-19 12:06:57 -0800 |
|---|---|---|
| committer | Pavan Kumar Chilamkurthi <pchilamk@codeaurora.org> | 2016-12-20 16:17:08 -0800 |
| commit | a50d444e11fbf3dbbbf3e9faa6272758c8844b07 (patch) | |
| tree | 2fe0a108a18c452874142f98ab59946001840ca4 | |
| parent | dd1c15c8028467a1122fff656e6c6cdc67a551bb (diff) | |
msm: camera: fd: Fix race condition during stream-off
Fix a race condition where stream off is triggerred
from userspace at the same time when FD driver
executing interrupt workqueue handler. If streamoff
is completed before workqueue handler, workqueue
handler may access memroy that was released
during streamoff which may lead to crash.
Change-Id: I9650f21d35ab6bfb9030848eac139ab1accc176e
CRs-Fixed: 1093918
Signed-off-by: Pavan Kumar Chilamkurthi <pchilamk@codeaurora.org>
4 files changed, 75 insertions, 45 deletions
diff --git a/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.c b/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.c index 31568da21fee..66eb3aebec83 100644 --- a/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.c +++ b/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.c @@ -364,7 +364,7 @@ static int msm_fd_vbif_error_handler(void *handle, uint32_t error) msm_fd_hw_get(fd, ctx->settings.speed); /* Get active buffer */ - active_buf = msm_fd_hw_get_active_buffer(fd); + active_buf = msm_fd_hw_get_active_buffer(fd, 1); if (active_buf == NULL) { dev_dbg(fd->dev, "no active buffer, return\n"); @@ -379,7 +379,7 @@ static int msm_fd_vbif_error_handler(void *handle, uint32_t error) msm_fd_hw_add_buffer(fd, active_buf); /* Schedule and restart */ - ret = msm_fd_hw_schedule_next_buffer(fd); + ret = msm_fd_hw_schedule_next_buffer(fd, 1); if (ret) { dev_err(fd->dev, "Cannot reschedule buffer, recovery failed\n"); fd->recovery_mode = 0; @@ -1221,11 +1221,12 @@ static void msm_fd_wq_handler(struct work_struct *work) int i; fd = container_of(work, struct msm_fd_device, work); - - active_buf = msm_fd_hw_get_active_buffer(fd); + MSM_FD_SPIN_LOCK(fd->slock, 1); + active_buf = msm_fd_hw_get_active_buffer(fd, 0); if (!active_buf) { /* This should never happen, something completely wrong */ dev_err(fd->dev, "Oops no active buffer empty queue\n"); + MSM_FD_SPIN_UNLOCK(fd->slock, 1); return; } ctx = vb2_get_drv_priv(active_buf->vb_v4l2_buf.vb2_buf.vb2_queue); @@ -1254,8 +1255,10 @@ static void msm_fd_wq_handler(struct work_struct *work) dev_dbg(fd->dev, "Got IRQ after Recovery\n"); } - /* We have the data from fd hw, we can start next processing */ - msm_fd_hw_schedule_next_buffer(fd); + if (fd->state == MSM_FD_DEVICE_RUNNING) { + /* We have the data from fd hw, we can start next processing */ + msm_fd_hw_schedule_next_buffer(fd, 0); + } /* Return buffer to vb queue */ active_buf->vb_v4l2_buf.sequence = ctx->fh.sequence; @@ -1270,8 +1273,12 @@ static void msm_fd_wq_handler(struct work_struct *work) fd_event->frame_id = ctx->sequence; v4l2_event_queue_fh(&ctx->fh, &event); - /* Release buffer from the device */ - msm_fd_hw_buffer_done(fd, active_buf); + if (fd->state == MSM_FD_DEVICE_RUNNING) { + /* Release buffer from the device */ + msm_fd_hw_buffer_done(fd, active_buf, 0); + } + + MSM_FD_SPIN_UNLOCK(fd->slock, 1); } /* diff --git a/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.h b/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.h index 7505f0585d42..6eae2b8d56fb 100644 --- a/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.h +++ b/drivers/media/platform/msm/camera_v2/fd/msm_fd_dev.h @@ -36,6 +36,18 @@ /* Max number of regulators defined in device tree */ #define MSM_FD_MAX_REGULATOR_NUM 3 +/* Conditional spin lock macro */ +#define MSM_FD_SPIN_LOCK(l, f) ({\ + if (f) \ + spin_lock(&l); \ +}) + +/* Conditional spin unlock macro */ +#define MSM_FD_SPIN_UNLOCK(l, f) ({ \ + if (f) \ + spin_unlock(&l); \ +}) + /* * struct msm_fd_size - Structure contain FD size related values. * @width: Image width. diff --git a/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.c b/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.c index 21d42a77accb..ea0db4d64c6d 100644 --- a/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.c +++ b/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.c @@ -1118,7 +1118,6 @@ static int msm_fd_hw_try_enable(struct msm_fd_device *fd, int enabled = 0; if (state == fd->state) { - fd->state = MSM_FD_DEVICE_RUNNING; atomic_set(&buffer->active, 1); @@ -1132,7 +1131,7 @@ static int msm_fd_hw_try_enable(struct msm_fd_device *fd, * msm_fd_hw_next_buffer - Get next buffer from fd device processing queue. * @fd: Fd device. */ -static struct msm_fd_buffer *msm_fd_hw_next_buffer(struct msm_fd_device *fd) +struct msm_fd_buffer *msm_fd_hw_get_next_buffer(struct msm_fd_device *fd) { struct msm_fd_buffer *buffer = NULL; @@ -1150,14 +1149,15 @@ static struct msm_fd_buffer *msm_fd_hw_next_buffer(struct msm_fd_device *fd) void msm_fd_hw_add_buffer(struct msm_fd_device *fd, struct msm_fd_buffer *buffer) { - spin_lock(&fd->slock); + MSM_FD_SPIN_LOCK(fd->slock, 1); atomic_set(&buffer->active, 0); init_completion(&buffer->completion); INIT_LIST_HEAD(&buffer->list); list_add_tail(&buffer->list, &fd->buf_queue); - spin_unlock(&fd->slock); + + MSM_FD_SPIN_UNLOCK(fd->slock, 1); } /* @@ -1173,8 +1173,7 @@ void msm_fd_hw_remove_buffers_from_queue(struct msm_fd_device *fd, struct msm_fd_buffer *active_buffer; unsigned long time; - spin_lock(&fd->slock); - + MSM_FD_SPIN_LOCK(fd->slock, 1); active_buffer = NULL; list_for_each_entry_safe(curr_buff, temp, &fd->buf_queue, list) { if (curr_buff->vb_v4l2_buf.vb2_buf.vb2_queue == vb2_q) { @@ -1189,21 +1188,31 @@ void msm_fd_hw_remove_buffers_from_queue(struct msm_fd_device *fd, } } } - spin_unlock(&fd->slock); + MSM_FD_SPIN_UNLOCK(fd->slock, 1); /* We need to wait active buffer to finish */ if (active_buffer) { time = wait_for_completion_timeout(&active_buffer->completion, msecs_to_jiffies(MSM_FD_PROCESSING_TIMEOUT_MS)); + + MSM_FD_SPIN_LOCK(fd->slock, 1); if (!time) { - /* Do a vb2 buffer done since it timed out */ - vb2_buffer_done(&active_buffer->vb_v4l2_buf.vb2_buf, - VB2_BUF_STATE_DONE); - /* Remove active buffer */ - msm_fd_hw_get_active_buffer(fd); - /* Schedule if other buffers are present in device */ - msm_fd_hw_schedule_next_buffer(fd); + if (atomic_read(&active_buffer->active)) { + atomic_set(&active_buffer->active, 0); + /* Do a vb2 buffer done since it timed out */ + vb2_buffer_done( + &active_buffer->vb_v4l2_buf.vb2_buf, + VB2_BUF_STATE_DONE); + /* Remove active buffer */ + msm_fd_hw_get_active_buffer(fd, 0); + /* Schedule if other buffers are present */ + msm_fd_hw_schedule_next_buffer(fd, 0); + } else { + dev_err(fd->dev, "activ buf no longer active\n"); + } } + fd->state = MSM_FD_DEVICE_IDLE; + MSM_FD_SPIN_UNLOCK(fd->slock, 1); } return; @@ -1215,22 +1224,19 @@ void msm_fd_hw_remove_buffers_from_queue(struct msm_fd_device *fd, * @buffer: Fd buffer. */ int msm_fd_hw_buffer_done(struct msm_fd_device *fd, - struct msm_fd_buffer *buffer) + struct msm_fd_buffer *buffer, u8 lock_flag) { int ret = 0; - - spin_lock(&fd->slock); + MSM_FD_SPIN_LOCK(fd->slock, lock_flag); if (atomic_read(&buffer->active)) { atomic_set(&buffer->active, 0); complete_all(&buffer->completion); } else { - dev_err(fd->dev, "Buffer is not active\n"); ret = -1; } - spin_unlock(&fd->slock); - + MSM_FD_SPIN_UNLOCK(fd->slock, lock_flag); return ret; } @@ -1238,17 +1244,18 @@ int msm_fd_hw_buffer_done(struct msm_fd_device *fd, * msm_fd_hw_get_active_buffer - Get active buffer from fd processing queue. * @fd: Fd device. */ -struct msm_fd_buffer *msm_fd_hw_get_active_buffer(struct msm_fd_device *fd) +struct msm_fd_buffer *msm_fd_hw_get_active_buffer(struct msm_fd_device *fd, + u8 lock_flag) { struct msm_fd_buffer *buffer = NULL; - spin_lock(&fd->slock); + MSM_FD_SPIN_LOCK(fd->slock, lock_flag); if (!list_empty(&fd->buf_queue)) { buffer = list_first_entry(&fd->buf_queue, struct msm_fd_buffer, list); list_del(&buffer->list); } - spin_unlock(&fd->slock); + MSM_FD_SPIN_UNLOCK(fd->slock, lock_flag); return buffer; } @@ -1263,12 +1270,13 @@ int msm_fd_hw_schedule_and_start(struct msm_fd_device *fd) { struct msm_fd_buffer *buf; - spin_lock(&fd->slock); - buf = msm_fd_hw_next_buffer(fd); + MSM_FD_SPIN_LOCK(fd->slock, 1); + + buf = msm_fd_hw_get_next_buffer(fd); if (buf) msm_fd_hw_try_enable(fd, buf, MSM_FD_DEVICE_IDLE); - spin_unlock(&fd->slock); + MSM_FD_SPIN_UNLOCK(fd->slock, 1); msm_fd_hw_update_settings(fd, buf); @@ -1281,26 +1289,26 @@ int msm_fd_hw_schedule_and_start(struct msm_fd_device *fd) * * NOTE: This can be executed only when device is in running state. */ -int msm_fd_hw_schedule_next_buffer(struct msm_fd_device *fd) +int msm_fd_hw_schedule_next_buffer(struct msm_fd_device *fd, u8 lock_flag) { struct msm_fd_buffer *buf; int ret; - spin_lock(&fd->slock); + MSM_FD_SPIN_LOCK(fd->slock, lock_flag); /* We can schedule next buffer only in running state */ if (fd->state != MSM_FD_DEVICE_RUNNING) { dev_err(fd->dev, "Can not schedule next buffer\n"); - spin_unlock(&fd->slock); + MSM_FD_SPIN_UNLOCK(fd->slock, lock_flag); return -EBUSY; } - buf = msm_fd_hw_next_buffer(fd); + buf = msm_fd_hw_get_next_buffer(fd); if (buf) { ret = msm_fd_hw_try_enable(fd, buf, MSM_FD_DEVICE_RUNNING); if (0 == ret) { - dev_err(fd->dev, "Ouch can not process next buffer\n"); - spin_unlock(&fd->slock); + dev_err(fd->dev, "Can not process next buffer\n"); + MSM_FD_SPIN_UNLOCK(fd->slock, lock_flag); return -EBUSY; } } else { @@ -1308,7 +1316,7 @@ int msm_fd_hw_schedule_next_buffer(struct msm_fd_device *fd) if (fd->recovery_mode) dev_err(fd->dev, "No Buffer in recovery mode.Device Idle\n"); } - spin_unlock(&fd->slock); + MSM_FD_SPIN_UNLOCK(fd->slock, lock_flag); msm_fd_hw_update_settings(fd, buf); diff --git a/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.h b/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.h index ea2e4cc1d117..62b381e5bfa6 100644 --- a/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.h +++ b/drivers/media/platform/msm/camera_v2/fd/msm_fd_hw.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2014-2015, The Linux Foundation. All rights reserved. +/* Copyright (c) 2014-2016, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -71,12 +71,15 @@ void msm_fd_hw_remove_buffers_from_queue(struct msm_fd_device *fd, struct vb2_queue *vb2_q); int msm_fd_hw_buffer_done(struct msm_fd_device *fd, - struct msm_fd_buffer *buffer); + struct msm_fd_buffer *buffer, u8 lock_flag); + +struct msm_fd_buffer *msm_fd_hw_get_active_buffer(struct msm_fd_device *fd, + u8 lock_flag); -struct msm_fd_buffer *msm_fd_hw_get_active_buffer(struct msm_fd_device *fd); +struct msm_fd_buffer *msm_fd_hw_get_next_buffer(struct msm_fd_device *fd); int msm_fd_hw_schedule_and_start(struct msm_fd_device *fd); -int msm_fd_hw_schedule_next_buffer(struct msm_fd_device *fd); +int msm_fd_hw_schedule_next_buffer(struct msm_fd_device *fd, u8 lock_flag); #endif /* __MSM_FD_HW_H__ */ |
