From 31d34d9bb30fef7aee1800ec63255083dbcdfa83 Mon Sep 17 00:00:00 2001 From: Sylwester Nawrocki Date: Thu, 26 Jul 2012 07:15:42 -0300 Subject: [PATCH] [media] s5p-fimc: Don't allocate fimc-capture video device dynamically This fixes potential invalid pointer de-reference, when media_entity_cleanup() is called before video device is unregistered. Signed-off-by: Sylwester Nawrocki Signed-off-by: Kyungmin Park Signed-off-by: Mauro Carvalho Chehab --- .../media/platform/s5p-fimc/fimc-capture.c | 28 ++++++------------- drivers/media/platform/s5p-fimc/fimc-core.h | 2 +- .../media/platform/s5p-fimc/fimc-mdevice.c | 2 +- .../media/platform/s5p-fimc/fimc-mdevice.h | 6 ++-- drivers/media/platform/s5p-fimc/fimc-reg.c | 6 ++-- 5 files changed, 16 insertions(+), 28 deletions(-) diff --git a/drivers/media/platform/s5p-fimc/fimc-capture.c b/drivers/media/platform/s5p-fimc/fimc-capture.c index 8e413dd3c0b0..5d3a70f5c5ca 100644 --- a/drivers/media/platform/s5p-fimc/fimc-capture.c +++ b/drivers/media/platform/s5p-fimc/fimc-capture.c @@ -304,7 +304,7 @@ int fimc_capture_resume(struct fimc_dev *fimc) INIT_LIST_HEAD(&fimc->vid_cap.active_buf_q); vid_cap->buf_index = 0; - fimc_pipeline_initialize(&fimc->pipeline, &vid_cap->vfd->entity, + fimc_pipeline_initialize(&fimc->pipeline, &vid_cap->vfd.entity, false); fimc_capture_hw_init(fimc); @@ -371,7 +371,7 @@ static int buffer_prepare(struct vb2_buffer *vb) unsigned long size = ctx->d_frame.payload[i]; if (vb2_plane_size(vb, i) < size) { - v4l2_err(ctx->fimc_dev->vid_cap.vfd, + v4l2_err(&ctx->fimc_dev->vid_cap.vfd, "User buffer too small (%ld < %ld)\n", vb2_plane_size(vb, i), size); return -EINVAL; @@ -503,7 +503,7 @@ static int fimc_capture_open(struct file *file) if (++fimc->vid_cap.refcnt == 1) { ret = fimc_pipeline_initialize(&fimc->pipeline, - &fimc->vid_cap.vfd->entity, true); + &fimc->vid_cap.vfd.entity, true); if (!ret && !fimc->vid_cap.user_subdev_api) ret = fimc_capture_set_default_format(fimc); @@ -1587,7 +1587,7 @@ static int fimc_capture_set_default_format(struct fimc_dev *fimc) static int fimc_register_capture_device(struct fimc_dev *fimc, struct v4l2_device *v4l2_dev) { - struct video_device *vfd; + struct video_device *vfd = &fimc->vid_cap.vfd; struct fimc_vid_cap *vid_cap; struct fimc_ctx *ctx; struct vb2_queue *q; @@ -1604,25 +1604,19 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, ctx->s_frame.fmt = fimc_find_format(NULL, NULL, FMT_FLAGS_CAM, 0); ctx->d_frame.fmt = ctx->s_frame.fmt; - vfd = video_device_alloc(); - if (!vfd) { - v4l2_err(v4l2_dev, "Failed to allocate video device\n"); - goto err_vd_alloc; - } - + memset(vfd, 0, sizeof(*vfd)); snprintf(vfd->name, sizeof(vfd->name), "fimc.%d.capture", fimc->id); vfd->fops = &fimc_capture_fops; vfd->ioctl_ops = &fimc_capture_ioctl_ops; vfd->v4l2_dev = v4l2_dev; vfd->minor = -1; - vfd->release = video_device_release; + vfd->release = video_device_release_empty; vfd->lock = &fimc->lock; video_set_drvdata(vfd, fimc); vid_cap = &fimc->vid_cap; - vid_cap->vfd = vfd; vid_cap->active_buf_cnt = 0; vid_cap->reqbufs_count = 0; vid_cap->refcnt = 0; @@ -1660,8 +1654,6 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, err_vd: media_entity_cleanup(&vfd->entity); err_ent: - video_device_release(vfd); -err_vd_alloc: kfree(ctx); return ret; } @@ -1691,12 +1683,10 @@ static void fimc_capture_subdev_unregistered(struct v4l2_subdev *sd) fimc_unregister_m2m_device(fimc); - if (fimc->vid_cap.vfd) { - media_entity_cleanup(&fimc->vid_cap.vfd->entity); - video_unregister_device(fimc->vid_cap.vfd); - fimc->vid_cap.vfd = NULL; + if (video_is_registered(&fimc->vid_cap.vfd)) { + video_unregister_device(&fimc->vid_cap.vfd); + media_entity_cleanup(&fimc->vid_cap.vfd.entity); } - kfree(fimc->vid_cap.ctx); fimc->vid_cap.ctx = NULL; } diff --git a/drivers/media/platform/s5p-fimc/fimc-core.h b/drivers/media/platform/s5p-fimc/fimc-core.h index 808ccc621846..30f93f2f2434 100644 --- a/drivers/media/platform/s5p-fimc/fimc-core.h +++ b/drivers/media/platform/s5p-fimc/fimc-core.h @@ -320,7 +320,7 @@ struct fimc_m2m_device { struct fimc_vid_cap { struct fimc_ctx *ctx; struct vb2_alloc_ctx *alloc_ctx; - struct video_device *vfd; + struct video_device vfd; struct v4l2_subdev subdev; struct media_pad vd_pad; struct v4l2_mbus_framefmt mf; diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.c b/drivers/media/platform/s5p-fimc/fimc-mdevice.c index 0dd26b69232c..3c76bd948fdb 100644 --- a/drivers/media/platform/s5p-fimc/fimc-mdevice.c +++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.c @@ -696,7 +696,7 @@ static int fimc_md_create_links(struct fimc_md *fmd) if (!fmd->fimc[i]) continue; source = &fmd->fimc[i]->vid_cap.subdev.entity; - sink = &fmd->fimc[i]->vid_cap.vfd->entity; + sink = &fmd->fimc[i]->vid_cap.vfd.entity; ret = media_entity_create_link(source, FIMC_SD_PAD_SOURCE, sink, 0, flags); if (ret) diff --git a/drivers/media/platform/s5p-fimc/fimc-mdevice.h b/drivers/media/platform/s5p-fimc/fimc-mdevice.h index 1f5dbaff5442..d310d9cc3e1a 100644 --- a/drivers/media/platform/s5p-fimc/fimc-mdevice.h +++ b/drivers/media/platform/s5p-fimc/fimc-mdevice.h @@ -99,14 +99,12 @@ static inline struct fimc_md *entity_to_fimc_mdev(struct media_entity *me) static inline void fimc_md_graph_lock(struct fimc_dev *fimc) { - BUG_ON(fimc->vid_cap.vfd == NULL); - mutex_lock(&fimc->vid_cap.vfd->entity.parent->graph_mutex); + mutex_lock(&fimc->vid_cap.vfd.entity.parent->graph_mutex); } static inline void fimc_md_graph_unlock(struct fimc_dev *fimc) { - BUG_ON(fimc->vid_cap.vfd == NULL); - mutex_unlock(&fimc->vid_cap.vfd->entity.parent->graph_mutex); + mutex_unlock(&fimc->vid_cap.vfd.entity.parent->graph_mutex); } int fimc_md_set_camclk(struct v4l2_subdev *sd, bool on); diff --git a/drivers/media/platform/s5p-fimc/fimc-reg.c b/drivers/media/platform/s5p-fimc/fimc-reg.c index 0e3eb9ce4f98..783408fd7d56 100644 --- a/drivers/media/platform/s5p-fimc/fimc-reg.c +++ b/drivers/media/platform/s5p-fimc/fimc-reg.c @@ -612,7 +612,7 @@ int fimc_hw_set_camera_source(struct fimc_dev *fimc, } if (i == ARRAY_SIZE(pix_desc)) { - v4l2_err(fimc->vid_cap.vfd, + v4l2_err(&fimc->vid_cap.vfd, "Camera color format not supported: %d\n", fimc->vid_cap.mf.code); return -EINVAL; @@ -684,7 +684,7 @@ int fimc_hw_set_camera_type(struct fimc_dev *fimc, cfg |= FIMC_REG_CIGCTRL_CAM_JPEG; break; default: - v4l2_err(vid_cap->vfd, + v4l2_err(&vid_cap->vfd, "Not supported camera pixel format: %#x\n", vid_cap->mf.code); return -EINVAL; @@ -701,7 +701,7 @@ int fimc_hw_set_camera_type(struct fimc_dev *fimc, cfg |= FIMC_REG_CIGCTRL_CAMIF_SELWB; break; default: - v4l2_err(vid_cap->vfd, "Invalid camera bus type selected\n"); + v4l2_err(&vid_cap->vfd, "Invalid camera bus type selected\n"); return -EINVAL; } writel(cfg, fimc->regs + FIMC_REG_CIGCTRL);