From ee68f004eca8d332195349533f67a7dbf552e843 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 04fbdbc6894c..ab80ad345d29 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 @@ -906,11 +906,9 @@ static int msm_isp_buf_done(struct msm_isp_buf_mgr *buf_mgr, if (state == MSM_ISP_BUFFER_STATE_DEQUEUED) { buf_info->state = MSM_ISP_BUFFER_STATE_DISPATCHED; spin_unlock_irqrestore(&bufq->bufq_lock, flags); - 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 { spin_unlock_irqrestore(&bufq->bufq_lock, flags); } 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 6a47a0dd130e..ce51c139fbae 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 @@ -72,6 +72,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 7cfda49645f8..789afd3e9f3b 100644 --- a/drivers/media/platform/msm/camera_v2/msm_sd.h +++ b/drivers/media/platform/msm/camera_v2/msm_sd.h @@ -75,7 +75,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 2b97f647b3cb..2689c829606e 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, @@ -269,7 +270,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; struct msm_vb2_buffer *msm_vb2; @@ -297,6 +299,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;