From 71f9c0f1e3414a17544b5a7fb4ef6e8702c9c21f Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 14 Oct 2024 16:52:41 +0200 Subject: media: v4l2-core: v4l2-dv-timings: check cvt/gtf result commit 9f070b1862f3411b8bcdfd51a8eaad25286f9deb upstream. The v4l2_detect_cvt/gtf functions should check the result against the timing capabilities: these functions calculate the timings, so if they are out of bounds, they should be rejected. To do this, add the struct v4l2_dv_timings_cap as argument to those functions. This required updates to the adv7604 and adv7842 drivers since the prototype of these functions has now changed. The timings struct that is passed to v4l2_detect_cvt/gtf in those two drivers is filled with the timings detected by the hardware. The vivid driver was also updated, but an additional check was added: the width and height specified by VIDIOC_S_DV_TIMINGS has to match the calculated result, otherwise something went wrong. Note that vivid *emulates* hardware, so all the values passed to the v4l2_detect_cvt/gtf functions came from the timings struct that was filled by userspace and passed on to the driver via VIDIOC_S_DV_TIMINGS. So these fields can contain random data. Both the constraints check via struct v4l2_dv_timings_cap and the additional width/height check ensure that the resulting timings are sane and not messed up by the v4l2_detect_cvt/gtf calculations. Signed-off-by: Hans Verkuil Fixes: 2576415846bc ("[media] v4l2: move dv-timings related code to v4l2-dv-timings.c") Cc: stable@vger.kernel.org Reported-by: syzbot+a828133770f62293563e@syzkaller.appspotmail.com Closes: https://lore.kernel.org/linux-media/000000000000013050062127830a@google.com/ Signed-off-by: Greg Kroah-Hartman --- drivers/media/i2c/adv7604.c | 5 +- drivers/media/i2c/adv7842.c | 13 +-- drivers/media/test-drivers/vivid/vivid-vid-cap.c | 15 ++- drivers/media/v4l2-core/v4l2-dv-timings.c | 132 ++++++++++++----------- 4 files changed, 95 insertions(+), 70 deletions(-) (limited to 'drivers') diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c index 96c2fa2c3b7a..d76ba25e0d1b 100644 --- a/drivers/media/i2c/adv7604.c +++ b/drivers/media/i2c/adv7604.c @@ -1405,12 +1405,13 @@ static int stdi2dv_timings(struct v4l2_subdev *sd, if (v4l2_detect_cvt(stdi->lcf + 1, hfreq, stdi->lcvs, 0, (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), - false, timings)) + false, adv76xx_get_dv_timings_cap(sd, -1), timings)) return 0; if (v4l2_detect_gtf(stdi->lcf + 1, hfreq, stdi->lcvs, (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), - false, state->aspect_ratio, timings)) + false, state->aspect_ratio, + adv76xx_get_dv_timings_cap(sd, -1), timings)) return 0; v4l2_dbg(2, debug, sd, diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index 7731cc1887e6..50245c2e0782 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -1431,14 +1431,15 @@ static int stdi2dv_timings(struct v4l2_subdev *sd, } if (v4l2_detect_cvt(stdi->lcf + 1, hfreq, stdi->lcvs, 0, - (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | - (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), - false, timings)) + (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | + (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), + false, adv7842_get_dv_timings_cap(sd), timings)) return 0; if (v4l2_detect_gtf(stdi->lcf + 1, hfreq, stdi->lcvs, - (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | - (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), - false, state->aspect_ratio, timings)) + (stdi->hs_pol == '+' ? V4L2_DV_HSYNC_POS_POL : 0) | + (stdi->vs_pol == '+' ? V4L2_DV_VSYNC_POS_POL : 0), + false, state->aspect_ratio, + adv7842_get_dv_timings_cap(sd), timings)) return 0; v4l2_dbg(2, debug, sd, diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c index 3864df45077d..b26b567f0c7f 100644 --- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c +++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c @@ -1719,12 +1719,19 @@ static bool valid_cvt_gtf_timings(struct v4l2_dv_timings *timings) h_freq = (u32)bt->pixelclock / total_h_pixel; if (bt->standards == 0 || (bt->standards & V4L2_DV_BT_STD_CVT)) { + struct v4l2_dv_timings cvt = {}; + if (v4l2_detect_cvt(total_v_lines, h_freq, bt->vsync, bt->width, - bt->polarities, bt->interlaced, timings)) + bt->polarities, bt->interlaced, + &vivid_dv_timings_cap, &cvt) && + cvt.bt.width == bt->width && cvt.bt.height == bt->height) { + *timings = cvt; return true; + } } if (bt->standards == 0 || (bt->standards & V4L2_DV_BT_STD_GTF)) { + struct v4l2_dv_timings gtf = {}; struct v4l2_fract aspect_ratio; find_aspect_ratio(bt->width, bt->height, @@ -1732,8 +1739,12 @@ static bool valid_cvt_gtf_timings(struct v4l2_dv_timings *timings) &aspect_ratio.denominator); if (v4l2_detect_gtf(total_v_lines, h_freq, bt->vsync, bt->polarities, bt->interlaced, - aspect_ratio, timings)) + aspect_ratio, &vivid_dv_timings_cap, + >f) && + gtf.bt.width == bt->width && gtf.bt.height == bt->height) { + *timings = gtf; return true; + } } return false; } diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c b/drivers/media/v4l2-core/v4l2-dv-timings.c index 942d0005c55e..2cf5dcee0ce8 100644 --- a/drivers/media/v4l2-core/v4l2-dv-timings.c +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c @@ -481,25 +481,28 @@ EXPORT_SYMBOL_GPL(v4l2_calc_timeperframe); * @polarities - the horizontal and vertical polarities (same as struct * v4l2_bt_timings polarities). * @interlaced - if this flag is true, it indicates interlaced format - * @fmt - the resulting timings. + * @cap - the v4l2_dv_timings_cap capabilities. + * @timings - the resulting timings. * * This function will attempt to detect if the given values correspond to a * valid CVT format. If so, then it will return true, and fmt will be filled * in with the found CVT timings. */ -bool v4l2_detect_cvt(unsigned frame_height, - unsigned hfreq, - unsigned vsync, - unsigned active_width, +bool v4l2_detect_cvt(unsigned int frame_height, + unsigned int hfreq, + unsigned int vsync, + unsigned int active_width, u32 polarities, bool interlaced, - struct v4l2_dv_timings *fmt) + const struct v4l2_dv_timings_cap *cap, + struct v4l2_dv_timings *timings) { - int v_fp, v_bp, h_fp, h_bp, hsync; - int frame_width, image_height, image_width; + struct v4l2_dv_timings t = {}; + int v_fp, v_bp, h_fp, h_bp, hsync; + int frame_width, image_height, image_width; bool reduced_blanking; bool rb_v2 = false; - unsigned pix_clk; + unsigned int pix_clk; if (vsync < 4 || vsync > 8) return false; @@ -625,36 +628,39 @@ bool v4l2_detect_cvt(unsigned frame_height, h_fp = h_blank - hsync - h_bp; } - fmt->type = V4L2_DV_BT_656_1120; - fmt->bt.polarities = polarities; - fmt->bt.width = image_width; - fmt->bt.height = image_height; - fmt->bt.hfrontporch = h_fp; - fmt->bt.vfrontporch = v_fp; - fmt->bt.hsync = hsync; - fmt->bt.vsync = vsync; - fmt->bt.hbackporch = frame_width - image_width - h_fp - hsync; + t.type = V4L2_DV_BT_656_1120; + t.bt.polarities = polarities; + t.bt.width = image_width; + t.bt.height = image_height; + t.bt.hfrontporch = h_fp; + t.bt.vfrontporch = v_fp; + t.bt.hsync = hsync; + t.bt.vsync = vsync; + t.bt.hbackporch = frame_width - image_width - h_fp - hsync; if (!interlaced) { - fmt->bt.vbackporch = frame_height - image_height - v_fp - vsync; - fmt->bt.interlaced = V4L2_DV_PROGRESSIVE; + t.bt.vbackporch = frame_height - image_height - v_fp - vsync; + t.bt.interlaced = V4L2_DV_PROGRESSIVE; } else { - fmt->bt.vbackporch = (frame_height - image_height - 2 * v_fp - + t.bt.vbackporch = (frame_height - image_height - 2 * v_fp - 2 * vsync) / 2; - fmt->bt.il_vbackporch = frame_height - image_height - 2 * v_fp - - 2 * vsync - fmt->bt.vbackporch; - fmt->bt.il_vfrontporch = v_fp; - fmt->bt.il_vsync = vsync; - fmt->bt.flags |= V4L2_DV_FL_HALF_LINE; - fmt->bt.interlaced = V4L2_DV_INTERLACED; + t.bt.il_vbackporch = frame_height - image_height - 2 * v_fp - + 2 * vsync - t.bt.vbackporch; + t.bt.il_vfrontporch = v_fp; + t.bt.il_vsync = vsync; + t.bt.flags |= V4L2_DV_FL_HALF_LINE; + t.bt.interlaced = V4L2_DV_INTERLACED; } - fmt->bt.pixelclock = pix_clk; - fmt->bt.standards = V4L2_DV_BT_STD_CVT; + t.bt.pixelclock = pix_clk; + t.bt.standards = V4L2_DV_BT_STD_CVT; if (reduced_blanking) - fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING; + t.bt.flags |= V4L2_DV_FL_REDUCED_BLANKING; + if (!v4l2_valid_dv_timings(&t, cap, NULL, NULL)) + return false; + *timings = t; return true; } EXPORT_SYMBOL_GPL(v4l2_detect_cvt); @@ -699,22 +705,25 @@ EXPORT_SYMBOL_GPL(v4l2_detect_cvt); * image height, so it has to be passed explicitly. Usually * the native screen aspect ratio is used for this. If it * is not filled in correctly, then 16:9 will be assumed. - * @fmt - the resulting timings. + * @cap - the v4l2_dv_timings_cap capabilities. + * @timings - the resulting timings. * * This function will attempt to detect if the given values correspond to a * valid GTF format. If so, then it will return true, and fmt will be filled * in with the found GTF timings. */ -bool v4l2_detect_gtf(unsigned frame_height, - unsigned hfreq, - unsigned vsync, - u32 polarities, - bool interlaced, - struct v4l2_fract aspect, - struct v4l2_dv_timings *fmt) +bool v4l2_detect_gtf(unsigned int frame_height, + unsigned int hfreq, + unsigned int vsync, + u32 polarities, + bool interlaced, + struct v4l2_fract aspect, + const struct v4l2_dv_timings_cap *cap, + struct v4l2_dv_timings *timings) { + struct v4l2_dv_timings t = {}; int pix_clk; - int v_fp, v_bp, h_fp, hsync; + int v_fp, v_bp, h_fp, hsync; int frame_width, image_height, image_width; bool default_gtf; int h_blank; @@ -783,36 +792,39 @@ bool v4l2_detect_gtf(unsigned frame_height, h_fp = h_blank / 2 - hsync; - fmt->type = V4L2_DV_BT_656_1120; - fmt->bt.polarities = polarities; - fmt->bt.width = image_width; - fmt->bt.height = image_height; - fmt->bt.hfrontporch = h_fp; - fmt->bt.vfrontporch = v_fp; - fmt->bt.hsync = hsync; - fmt->bt.vsync = vsync; - fmt->bt.hbackporch = frame_width - image_width - h_fp - hsync; + t.type = V4L2_DV_BT_656_1120; + t.bt.polarities = polarities; + t.bt.width = image_width; + t.bt.height = image_height; + t.bt.hfrontporch = h_fp; + t.bt.vfrontporch = v_fp; + t.bt.hsync = hsync; + t.bt.vsync = vsync; + t.bt.hbackporch = frame_width - image_width - h_fp - hsync; if (!interlaced) { - fmt->bt.vbackporch = frame_height - image_height - v_fp - vsync; - fmt->bt.interlaced = V4L2_DV_PROGRESSIVE; + t.bt.vbackporch = frame_height - image_height - v_fp - vsync; + t.bt.interlaced = V4L2_DV_PROGRESSIVE; } else { - fmt->bt.vbackporch = (frame_height - image_height - 2 * v_fp - + t.bt.vbackporch = (frame_height - image_height - 2 * v_fp - 2 * vsync) / 2; - fmt->bt.il_vbackporch = frame_height - image_height - 2 * v_fp - - 2 * vsync - fmt->bt.vbackporch; - fmt->bt.il_vfrontporch = v_fp; - fmt->bt.il_vsync = vsync; - fmt->bt.flags |= V4L2_DV_FL_HALF_LINE; - fmt->bt.interlaced = V4L2_DV_INTERLACED; + t.bt.il_vbackporch = frame_height - image_height - 2 * v_fp - + 2 * vsync - t.bt.vbackporch; + t.bt.il_vfrontporch = v_fp; + t.bt.il_vsync = vsync; + t.bt.flags |= V4L2_DV_FL_HALF_LINE; + t.bt.interlaced = V4L2_DV_INTERLACED; } - fmt->bt.pixelclock = pix_clk; - fmt->bt.standards = V4L2_DV_BT_STD_GTF; + t.bt.pixelclock = pix_clk; + t.bt.standards = V4L2_DV_BT_STD_GTF; if (!default_gtf) - fmt->bt.flags |= V4L2_DV_FL_REDUCED_BLANKING; + t.bt.flags |= V4L2_DV_FL_REDUCED_BLANKING; + if (!v4l2_valid_dv_timings(&t, cap, NULL, NULL)) + return false; + *timings = t; return true; } EXPORT_SYMBOL_GPL(v4l2_detect_gtf); -- cgit v1.2.3