From a805985a97fbff383f1ce607bd79c4d5a3c3f3f0 Mon Sep 17 00:00:00 2001 From: Lakshmi Narayana Kalavala Date: Fri, 4 Dec 2015 16:27:43 -0800 Subject: [PATCH] msm: camera: Fix memory corruption with vb2 buffers The camera generic buffer manager and isp buffer manager keep references of vb2 buffers locally during buffer circulation. If for some reason the vb2 buffers are freed from a cleanup call from mediaserver. The memory for the buffers is freed. But the camera buffer managers still access them for a fraction of time before the cleanup call is triggered from daemon process. Hence make sure to access the vb2 buffers only after checking for the existence in vb2 queues to avoid memory corruption. Change-Id: I7a1e5f9a3af3345e0c37d3208facbab107a6b9ed Signed-off-by: Lakshmi Narayana Kalavala --- .../platform/msm/camera_v2/isp/msm_buf_mgr.c | 6 ++---- .../camera_v2/msm_buf_mgr/msm_generic_buf_mgr.c | 16 +++++++++------- .../camera_v2/msm_buf_mgr/msm_generic_buf_mgr.h | 1 + drivers/media/platform/msm/camera_v2/msm_sd.h | 3 ++- .../platform/msm/camera_v2/msm_vb2/msm_vb2.c | 7 ++++++- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/drivers/media/platform/msm/camera_v2/isp/msm_buf_mgr.c b/drivers/media/platform/msm/camera_v2/isp/msm_buf_mgr.c index 3f8152baa9da..f15fe2682921 100644 --- a/drivers/media/platform/msm/camera_v2/isp/msm_buf_mgr.c +++ b/drivers/media/platform/msm/camera_v2/isp/msm_buf_mgr.c @@ -921,11 +921,9 @@ static int msm_isp_buf_done(struct msm_isp_buf_mgr *buf_mgr, buf_info->state = MSM_ISP_BUFFER_STATE_DISPATCHED; spin_unlock_irqrestore(&bufq->bufq_lock, flags); if (MSM_ISP_BUFFER_SRC_HAL == BUF_SRC(bufq->stream_id)) { - buf_info->vb2_buf->v4l2_buf.timestamp = *tv; - buf_info->vb2_buf->v4l2_buf.sequence = frame_id; - buf_info->vb2_buf->v4l2_buf.reserved = output_format; buf_mgr->vb2_ops->buf_done(buf_info->vb2_buf, - bufq->session_id, bufq->stream_id); + bufq->session_id, bufq->stream_id, + frame_id, tv, output_format); } else { pr_err("%s: Error wrong buf done %d\n", __func__, state); diff --git a/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.c b/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.c index 18436734f4f8..7397f217a6b8 100644 --- a/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.c +++ b/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.c @@ -45,6 +45,7 @@ static int32_t msm_buf_mngr_get_buf(struct msm_buf_mngr_device *dev, } new_entry->session_id = buf_info->session_id; new_entry->stream_id = buf_info->stream_id; + new_entry->index = new_entry->vb2_buf->v4l2_buf.index; spin_lock_irqsave(&dev->buf_q_spinlock, flags); list_add_tail(&new_entry->entry, &dev->buf_qhead); spin_unlock_irqrestore(&dev->buf_q_spinlock, flags); @@ -101,14 +102,14 @@ static int32_t msm_buf_mngr_buf_done(struct msm_buf_mngr_device *buf_mngr_dev, list_for_each_entry_safe(bufs, save, &buf_mngr_dev->buf_qhead, entry) { if ((bufs->session_id == buf_info->session_id) && (bufs->stream_id == buf_info->stream_id) && - (bufs->vb2_buf->v4l2_buf.index == buf_info->index)) { - bufs->vb2_buf->v4l2_buf.sequence = buf_info->frame_id; - bufs->vb2_buf->v4l2_buf.timestamp = buf_info->timestamp; - bufs->vb2_buf->v4l2_buf.reserved = buf_info->reserved; + (bufs->index == buf_info->index)) { ret = buf_mngr_dev->vb2_ops.buf_done (bufs->vb2_buf, buf_info->session_id, - buf_info->stream_id); + buf_info->stream_id, + buf_info->frame_id, + &buf_info->timestamp, + buf_info->reserved); list_del_init(&bufs->entry); kfree(bufs); break; @@ -130,7 +131,7 @@ static int32_t msm_buf_mngr_put_buf(struct msm_buf_mngr_device *buf_mngr_dev, list_for_each_entry_safe(bufs, save, &buf_mngr_dev->buf_qhead, entry) { if ((bufs->session_id == buf_info->session_id) && (bufs->stream_id == buf_info->stream_id) && - (bufs->vb2_buf->v4l2_buf.index == buf_info->index)) { + (bufs->index == buf_info->index)) { ret = buf_mngr_dev->vb2_ops.put_buf(bufs->vb2_buf, buf_info->session_id, buf_info->stream_id); list_del_init(&bufs->entry); @@ -149,6 +150,7 @@ static int32_t msm_generic_buf_mngr_flush( unsigned long flags; struct msm_get_bufs *bufs, *save; int32_t ret = -EINVAL; + struct timeval ts; spin_lock_irqsave(&buf_mngr_dev->buf_q_spinlock, flags); /* @@ -160,7 +162,7 @@ static int32_t msm_generic_buf_mngr_flush( (bufs->stream_id == buf_info->stream_id)) { ret = buf_mngr_dev->vb2_ops.buf_done(bufs->vb2_buf, buf_info->session_id, - buf_info->stream_id); + buf_info->stream_id, 0, &ts, 0); pr_err("Bufs not flushed: str_id = %d buf_index = %d ret = %d\n", buf_info->stream_id, bufs->vb2_buf->v4l2_buf.index, ret); diff --git a/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.h b/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.h index af9f927cb3b4..5b8fca295feb 100644 --- a/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.h +++ b/drivers/media/platform/msm/camera_v2/msm_buf_mgr/msm_generic_buf_mgr.h @@ -29,6 +29,7 @@ struct msm_get_bufs { struct vb2_buffer *vb2_buf; uint32_t session_id; uint32_t stream_id; + uint32_t index; }; struct msm_buf_mngr_device { diff --git a/drivers/media/platform/msm/camera_v2/msm_sd.h b/drivers/media/platform/msm/camera_v2/msm_sd.h index d8f337c89e5d..01b37085c980 100644 --- a/drivers/media/platform/msm/camera_v2/msm_sd.h +++ b/drivers/media/platform/msm/camera_v2/msm_sd.h @@ -72,7 +72,8 @@ struct msm_sd_req_vb2_q { int (*put_buf)(struct vb2_buffer *vb2_buf, int session_id, unsigned int stream_id); int (*buf_done)(struct vb2_buffer *vb2_buf, int session_id, - unsigned int stream_id); + unsigned int stream_id, uint32_t sequence, struct timeval *ts, + uint32_t reserved); int (*flush_buf)(int session_id, unsigned int stream_id); }; diff --git a/drivers/media/platform/msm/camera_v2/msm_vb2/msm_vb2.c b/drivers/media/platform/msm/camera_v2/msm_vb2/msm_vb2.c index 1724c27adc64..367a0bf56877 100644 --- a/drivers/media/platform/msm/camera_v2/msm_vb2/msm_vb2.c +++ b/drivers/media/platform/msm/camera_v2/msm_vb2/msm_vb2.c @@ -10,6 +10,7 @@ * GNU General Public License for more details. */ +#define pr_fmt(fmt) "CAM-VB2 %s:%d " fmt, __func__, __LINE__ #include "msm_vb2.h" static int msm_vb2_queue_setup(struct vb2_queue *q, @@ -347,7 +348,8 @@ static int msm_vb2_put_buf(struct vb2_buffer *vb, int session_id, } static int msm_vb2_buf_done(struct vb2_buffer *vb, int session_id, - unsigned int stream_id) + unsigned int stream_id, uint32_t sequence, + struct timeval *ts, uint32_t reserved) { unsigned long flags, rl_flags; struct msm_vb2_buffer *msm_vb2; @@ -386,6 +388,9 @@ static int msm_vb2_buf_done(struct vb2_buffer *vb, int session_id, container_of(vb, struct msm_vb2_buffer, vb2_buf); /* put buf before buf done */ if (msm_vb2->in_freeq) { + vb->v4l2_buf.sequence = sequence; + vb->v4l2_buf.timestamp = *ts; + vb->v4l2_buf.reserved = reserved; vb2_buffer_done(vb, VB2_BUF_STATE_DONE); msm_vb2->in_freeq = 0; rc = 0;