From f8bcaf714abfc94818dff8c0db84d750433984f4 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 12 Oct 2022 15:32:28 +0100 Subject: [PATCH 1/8] media: vivid: s_fbuf: add more sanity checks VIDIOC_S_FBUF is by definition a scary ioctl, which is why only root can use it. But at least check if the framebuffer parameters match that of one of the framebuffer created by vivid, and reject anything else. Signed-off-by: Hans Verkuil Fixes: ef834f7836ec ([media] vivid: add the video capture and output parts) Signed-off-by: Mauro Carvalho Chehab --- drivers/media/test-drivers/vivid/vivid-core.c | 22 +++++++++++++++++++ drivers/media/test-drivers/vivid/vivid-core.h | 2 ++ .../media/test-drivers/vivid/vivid-vid-cap.c | 9 +++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c index 04b75666bad4..61d48fbc3d15 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.c +++ b/drivers/media/test-drivers/vivid/vivid-core.c @@ -339,6 +339,28 @@ static int vidioc_g_fbuf(struct file *file, void *fh, struct v4l2_framebuffer *a return vivid_vid_out_g_fbuf(file, fh, a); } +/* + * Only support the framebuffer of one of the vivid instances. + * Anything else is rejected. + */ +bool vivid_validate_fb(const struct v4l2_framebuffer *a) +{ + struct vivid_dev *dev; + int i; + + for (i = 0; i < n_devs; i++) { + dev = vivid_devs[i]; + if (!dev || !dev->video_pbase) + continue; + if ((unsigned long)a->base == dev->video_pbase && + a->fmt.width <= dev->display_width && + a->fmt.height <= dev->display_height && + a->fmt.bytesperline <= dev->display_byte_stride) + return true; + } + return false; +} + static int vidioc_s_fbuf(struct file *file, void *fh, const struct v4l2_framebuffer *a) { struct video_device *vdev = video_devdata(file); diff --git a/drivers/media/test-drivers/vivid/vivid-core.h b/drivers/media/test-drivers/vivid/vivid-core.h index bfcfb3515901..473f3598db5a 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.h +++ b/drivers/media/test-drivers/vivid/vivid-core.h @@ -613,4 +613,6 @@ static inline bool vivid_is_hdmi_out(const struct vivid_dev *dev) return dev->output_type[dev->output] == HDMI; } +bool vivid_validate_fb(const struct v4l2_framebuffer *a); + #endif diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c index 86b158eeb2d8..e3e78b5bd227 100644 --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c @@ -1276,7 +1276,14 @@ int vivid_vid_cap_s_fbuf(struct file *file, void *fh, return -EINVAL; if (a->fmt.bytesperline < (a->fmt.width * fmt->bit_depth[0]) / 8) return -EINVAL; - if (a->fmt.height * a->fmt.bytesperline < a->fmt.sizeimage) + if (a->fmt.bytesperline > a->fmt.sizeimage / a->fmt.height) + return -EINVAL; + + /* + * Only support the framebuffer of one of the vivid instances. + * Anything else is rejected. + */ + if (!vivid_validate_fb(a)) return -EINVAL; dev->fb_vbase_cap = phys_to_virt((unsigned long)a->base); From 1f65ea411cc7b6ff128d82a3493d7b5648054e6f Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Thu, 13 Oct 2022 15:18:46 +0100 Subject: [PATCH 2/8] media: vivid: dev->bitmap_cap wasn't freed in all cases Whenever the compose width/height values change, the dev->bitmap_cap vmalloc'ed array must be freed and dev->bitmap_cap set to NULL. This was done in some places, but not all. This is only an issue if overlay support is enabled and the bitmap clipping is used. Signed-off-by: Hans Verkuil Fixes: ef834f7836ec ([media] vivid: add the video capture and output parts) Signed-off-by: Mauro Carvalho Chehab --- .../media/test-drivers/vivid/vivid-vid-cap.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c index e3e78b5bd227..d52d24b61d34 100644 --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c @@ -453,6 +453,12 @@ void vivid_update_format_cap(struct vivid_dev *dev, bool keep_controls) tpg_reset_source(&dev->tpg, dev->src_rect.width, dev->src_rect.height, dev->field_cap); dev->crop_cap = dev->src_rect; dev->crop_bounds_cap = dev->src_rect; + if (dev->bitmap_cap && + (dev->compose_cap.width != dev->crop_cap.width || + dev->compose_cap.height != dev->crop_cap.height)) { + vfree(dev->bitmap_cap); + dev->bitmap_cap = NULL; + } dev->compose_cap = dev->crop_cap; if (V4L2_FIELD_HAS_T_OR_B(dev->field_cap)) dev->compose_cap.height /= 2; @@ -913,6 +919,8 @@ int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection struct vivid_dev *dev = video_drvdata(file); struct v4l2_rect *crop = &dev->crop_cap; struct v4l2_rect *compose = &dev->compose_cap; + unsigned orig_compose_w = compose->width; + unsigned orig_compose_h = compose->height; unsigned factor = V4L2_FIELD_HAS_T_OR_B(dev->field_cap) ? 2 : 1; int ret; @@ -1029,17 +1037,17 @@ int vivid_vid_cap_s_selection(struct file *file, void *fh, struct v4l2_selection s->r.height /= factor; } v4l2_rect_map_inside(&s->r, &dev->fmt_cap_rect); - if (dev->bitmap_cap && (compose->width != s->r.width || - compose->height != s->r.height)) { - vfree(dev->bitmap_cap); - dev->bitmap_cap = NULL; - } *compose = s->r; break; default: return -EINVAL; } + if (dev->bitmap_cap && (compose->width != orig_compose_w || + compose->height != orig_compose_h)) { + vfree(dev->bitmap_cap); + dev->bitmap_cap = NULL; + } tpg_s_crop_compose(&dev->tpg, crop, compose); return 0; } From 4b6d66a45ed34a15721cb9e11492fa1a24bc83df Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Thu, 13 Oct 2022 09:00:34 +0100 Subject: [PATCH 3/8] media: v4l2-dv-timings: add sanity checks for blanking values Add sanity checks to v4l2_valid_dv_timings() to ensure that the provided blanking values are reasonable. Signed-off-by: Hans Verkuil Fixes: b18787ed1ce3 ([media] v4l2-dv-timings: add new helper module) Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-dv-timings.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c index af48705c704f..003c32fed3f7 100644 --- a/drivers/media/v4l2-core/v4l2-dv-timings.c +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c @@ -161,6 +161,20 @@ bool v4l2_valid_dv_timings(const struct v4l2_dv_timings *t, (bt->interlaced && !(caps & V4L2_DV_BT_CAP_INTERLACED)) || (!bt->interlaced && !(caps & V4L2_DV_BT_CAP_PROGRESSIVE))) return false; + + /* sanity checks for the blanking timings */ + if (!bt->interlaced && + (bt->il_vbackporch || bt->il_vsync || bt->il_vfrontporch)) + return false; + if (bt->hfrontporch > 2 * bt->width || + bt->hsync > 1024 || bt->hbackporch > 1024) + return false; + if (bt->vfrontporch > 4096 || + bt->vsync > 128 || bt->vbackporch > 4096) + return false; + if (bt->interlaced && (bt->il_vfrontporch > 4096 || + bt->il_vsync > 128 || bt->il_vbackporch > 4096)) + return false; return fnc == NULL || fnc(t, fnc_handle); } EXPORT_SYMBOL_GPL(v4l2_valid_dv_timings); From 8da7f0976b9071b528c545008de9d10cc81883b1 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 12 Oct 2022 16:46:17 +0100 Subject: [PATCH 4/8] media: videodev2.h: V4L2_DV_BT_BLANKING_HEIGHT should check 'interlaced' If it is a progressive (non-interlaced) format, then ignore the interlaced timing values. Signed-off-by: Hans Verkuil Fixes: 7f68127fa11f ([media] videodev2.h: defines to calculate blanking and frame sizes) Signed-off-by: Mauro Carvalho Chehab --- include/uapi/linux/videodev2.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index 86cae23cc446..29da1f4b4578 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -1601,7 +1601,8 @@ struct v4l2_bt_timings { ((bt)->width + V4L2_DV_BT_BLANKING_WIDTH(bt)) #define V4L2_DV_BT_BLANKING_HEIGHT(bt) \ ((bt)->vfrontporch + (bt)->vsync + (bt)->vbackporch + \ - (bt)->il_vfrontporch + (bt)->il_vsync + (bt)->il_vbackporch) + ((bt)->interlaced ? \ + ((bt)->il_vfrontporch + (bt)->il_vsync + (bt)->il_vbackporch) : 0)) #define V4L2_DV_BT_FRAME_HEIGHT(bt) \ ((bt)->height + V4L2_DV_BT_BLANKING_HEIGHT(bt)) From eb1d969203eb8212741751f88dcf5cb56bb11830 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Fri, 21 Oct 2022 13:21:25 +0100 Subject: [PATCH 5/8] media: vivid: fix control handler mutex deadlock vivid_update_format_cap() can be called from an s_ctrl callback. In that case (keep_controls == true) no control framework functions can be called that take the control handler mutex. The new call to v4l2_ctrl_modify_dimensions() did exactly that. Signed-off-by: Hans Verkuil Fixes: 6bc7643d1b9c (media: vivid: add pixel_array test control) Signed-off-by: Mauro Carvalho Chehab --- drivers/media/test-drivers/vivid/vivid-vid-cap.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c index d52d24b61d34..11620eaf941e 100644 --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c @@ -466,6 +466,14 @@ void vivid_update_format_cap(struct vivid_dev *dev, bool keep_controls) tpg_s_video_aspect(&dev->tpg, vivid_get_video_aspect(dev)); tpg_s_pixel_aspect(&dev->tpg, vivid_get_pixel_aspect(dev)); tpg_update_mv_step(&dev->tpg); + + /* + * We can be called from within s_ctrl, in that case we can't + * modify controls. Luckily we don't need to in that case. + */ + if (keep_controls) + return; + dims[0] = roundup(dev->src_rect.width, PIXEL_ARRAY_DIV); dims[1] = roundup(dev->src_rect.height, PIXEL_ARRAY_DIV); v4l2_ctrl_modify_dimensions(dev->pixel_array, dims); From 957148e22870e4021082dddd12f8619a49d9bb50 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 12 Oct 2022 15:31:51 +0100 Subject: [PATCH 6/8] media: vivid: drop GFP_DMA32 >From what I can see, this is not needed. And since using it issues a 'deprecated' warning, just drop it. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/test-drivers/vivid/vivid-osd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/test-drivers/vivid/vivid-osd.c b/drivers/media/test-drivers/vivid/vivid-osd.c index fbaec8acc161..ec25edc679b3 100644 --- a/drivers/media/test-drivers/vivid/vivid-osd.c +++ b/drivers/media/test-drivers/vivid/vivid-osd.c @@ -357,7 +357,7 @@ int vivid_fb_init(struct vivid_dev *dev) int ret; dev->video_buffer_size = MAX_OSD_HEIGHT * MAX_OSD_WIDTH * 2; - dev->video_vbase = kzalloc(dev->video_buffer_size, GFP_KERNEL | GFP_DMA32); + dev->video_vbase = kzalloc(dev->video_buffer_size, GFP_KERNEL); if (dev->video_vbase == NULL) return -ENOMEM; dev->video_pbase = virt_to_phys(dev->video_vbase); From 69d78a80da4ef12faf2a6f9cfa2097ab4ac43983 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 17 Oct 2022 15:09:06 +0100 Subject: [PATCH 7/8] media: vivid: set num_in/outputs to 0 if not supported If node_types does not have video/vbi/meta inputs or outputs, then set num_inputs/num_outputs to 0 instead of 1. Signed-off-by: Hans Verkuil Fixes: 0c90f649d2f5 (media: vivid: add vivid_create_queue() helper) Signed-off-by: Mauro Carvalho Chehab --- drivers/media/test-drivers/vivid/vivid-core.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c index 61d48fbc3d15..f28440e6c9f8 100644 --- a/drivers/media/test-drivers/vivid/vivid-core.c +++ b/drivers/media/test-drivers/vivid/vivid-core.c @@ -942,8 +942,12 @@ static int vivid_detect_feature_set(struct vivid_dev *dev, int inst, /* how many inputs do we have and of what type? */ dev->num_inputs = num_inputs[inst]; - if (dev->num_inputs < 1) - dev->num_inputs = 1; + if (node_type & 0x20007) { + if (dev->num_inputs < 1) + dev->num_inputs = 1; + } else { + dev->num_inputs = 0; + } if (dev->num_inputs >= MAX_INPUTS) dev->num_inputs = MAX_INPUTS; for (i = 0; i < dev->num_inputs; i++) { @@ -960,8 +964,12 @@ static int vivid_detect_feature_set(struct vivid_dev *dev, int inst, /* how many outputs do we have and of what type? */ dev->num_outputs = num_outputs[inst]; - if (dev->num_outputs < 1) - dev->num_outputs = 1; + if (node_type & 0x40300) { + if (dev->num_outputs < 1) + dev->num_outputs = 1; + } else { + dev->num_outputs = 0; + } if (dev->num_outputs >= MAX_OUTPUTS) dev->num_outputs = MAX_OUTPUTS; for (i = 0; i < dev->num_outputs; i++) { From de547896aac606a00435a219757a940ece142bf0 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 17 Oct 2022 15:15:05 +0100 Subject: [PATCH 8/8] media: vivid.rst: loop_video is set on the capture devnode The example on how to use and test Capture Overlay specified the wrong video device node. Back in 2015 the loop_video control moved from the output device to the capture device, but this example code is still referring to the output video device. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- Documentation/admin-guide/media/vivid.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/admin-guide/media/vivid.rst b/Documentation/admin-guide/media/vivid.rst index 4f680dc9661c..abd90ed31090 100644 --- a/Documentation/admin-guide/media/vivid.rst +++ b/Documentation/admin-guide/media/vivid.rst @@ -1318,7 +1318,7 @@ instance. This setup would require the following commands: $ v4l2-ctl -d2 -i2 $ v4l2-ctl -d2 -c horizontal_movement=4 $ v4l2-ctl -d1 --overlay=1 - $ v4l2-ctl -d1 -c loop_video=1 + $ v4l2-ctl -d0 -c loop_video=1 $ v4l2-ctl -d2 --stream-mmap --overlay=1 And from another console: