[PATCH v2] media: cobalt: bound the dv timings to the descriptor buffers

[PATCH v2] media: cobalt: bound the dv timings to the descriptor buffers

[Guo Zihao]

The DMA descriptor buffers are sized from the maximum frame the driver
supports:

        const size_t max_pages_per_line =
                (COBALT_MAX_WIDTH * COBALT_MAX_BPP) / PAGE_SIZE + 2;
        const size_t bytes =
                COBALT_MAX_HEIGHT * max_pages_per_line * 0x20;

With COBALT_MAX_WIDTH 1920, COBALT_MAX_HEIGHT 1200 and COBALT_MAX_BPP 3
that is 115200 bytes, or 3600 descriptors of 0x20 bytes each.

cobalt_s_dv_timings() however records whatever the subdevice accepts
without checking it against those maxima:

        err = v4l2_subdev_call(s->sd, pad, s_dv_timings, 0, timings);
        if (!err) {
                s->timings = *timings;
                s->width = timings->bt.width;
                s->height = timings->bt.height;
                s->stride = timings->bt.width * s->bpp;
        }

descriptor_list_create() in cobalt-omnitek.c then walks the whole frame
and writes one descriptor per scatterlist segment, with no upper bound of
its own. An HDMI source at 4096x2160 with bpp 3 gives stride 12288 and
size 26542080, which needs far more than the 3600 descriptors the buffer
holds, so d[] walks off the end of the coherent allocation.

cobalt_try_fmt_vid_cap() and cobalt_try_fmt_vid_out() already cap width
and height at 1920x1080, and this patch adds the same bound for the
timings path plus a stride limit on the pixelformat path.

No Fixes tag. The descriptor sizing and s_dv_timings() both come from the
initial driver, 85756a069c55 ("[media] cobalt: add new driver").

Reviewed-by: Liu Chao <liuc63@xiaopeng.com>
Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>
---
v2: declare the local struct cobalt pointer in cobalt_s_dv_timings().
    The cobalt_info() macro expands to v4l2_info(&cobalt->v4l2_dev, ...),
    so the function needs a variable of that name, which the previous
    version was missing. Found by the kernel test robot.

 drivers/media/pci/cobalt/cobalt-v4l2.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
index 51fd9576c..8ffee8ed6 100644
--- a/drivers/media/pci/cobalt/cobalt-v4l2.c
+++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
@@ -617,6 +617,7 @@ static int cobalt_s_dv_timings(struct file *file, void *priv,
 				    struct v4l2_dv_timings *timings)
 {
 	struct cobalt_stream *s = video_drvdata(file);
+	struct cobalt *cobalt = s->cobalt;
 	int err;
 
 	if (s->input == 1) {
@@ -630,6 +631,13 @@ static int cobalt_s_dv_timings(struct file *file, void *priv,
 	if (vb2_is_busy(&s->q))
 		return -EBUSY;
 
+	if (timings->bt.width > COBALT_MAX_WIDTH ||
+	    timings->bt.height > COBALT_MAX_HEIGHT) {
+		cobalt_info("timings %ux%u out of range\n",
+			    timings->bt.width, timings->bt.height);
+		return -EINVAL;
+	}
+
 	err = v4l2_subdev_call(s->sd,
 			pad, s_dv_timings, 0, timings);
 	if (!err) {
@@ -781,6 +789,14 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv,
 		break;
 	}
 
+	/*
+	 * The DMA descriptor buffers are sized for at most
+	 * COBALT_MAX_WIDTH x COBALT_MAX_HEIGHT, so limit the line stride
+	 * accordingly.
+	 */
+	if (pix->bytesperline > COBALT_MAX_WIDTH * COBALT_MAX_BPP)
+		pix->bytesperline = COBALT_MAX_WIDTH * COBALT_MAX_BPP;
+
 	pix->sizeimage = pix->bytesperline * pix->height;
 	pix->field = V4L2_FIELD_NONE;
 
-- 
2.50.1

Re: [PATCH v2] media: cobalt: bound the dv timings to the descriptor buffers

[Hans Verkuil]

On 20/09/2026 03:59, Guo Zihao wrote:
> The DMA descriptor buffers are sized from the maximum frame the driver
> supports:
> 
>         const size_t max_pages_per_line =
>                 (COBALT_MAX_WIDTH * COBALT_MAX_BPP) / PAGE_SIZE + 2;
>         const size_t bytes =
>                 COBALT_MAX_HEIGHT * max_pages_per_line * 0x20;
> 
> With COBALT_MAX_WIDTH 1920, COBALT_MAX_HEIGHT 1200 and COBALT_MAX_BPP 3
> that is 115200 bytes, or 3600 descriptors of 0x20 bytes each.
> 
> cobalt_s_dv_timings() however records whatever the subdevice accepts
> without checking it against those maxima:
> 
>         err = v4l2_subdev_call(s->sd, pad, s_dv_timings, 0, timings);
>         if (!err) {
>                 s->timings = *timings;
>                 s->width = timings->bt.width;
>                 s->height = timings->bt.height;
>                 s->stride = timings->bt.width * s->bpp;
>         }
> 
> descriptor_list_create() in cobalt-omnitek.c then walks the whole frame
> and writes one descriptor per scatterlist segment, with no upper bound of
> its own. An HDMI source at 4096x2160 with bpp 3 gives stride 12288 and
> size 26542080, which needs far more than the 3600 descriptors the buffer
> holds, so d[] walks off the end of the coherent allocation.
> 
> cobalt_try_fmt_vid_cap() and cobalt_try_fmt_vid_out() already cap width
> and height at 1920x1080, and this patch adds the same bound for the
> timings path plus a stride limit on the pixelformat path.
> 
> No Fixes tag. The descriptor sizing and s_dv_timings() both come from the
> initial driver, 85756a069c55 ("[media] cobalt: add new driver").
> 
> Reviewed-by: Liu Chao <liuc63@xiaopeng.com>
> Signed-off-by: Guo Zihao <guozh23@xiaopeng.com>

Rejected-by: Hans Verkuil <hverkuil+cisco@kernel.org>

The dv timings are checked in the subdevices code, so there is no
need to add the check here.

And bytesperline can definitely be larger than the width of the captured
video: in that case it will be composed into the larger buffer.

Regards,

	Hans

> ---
> v2: declare the local struct cobalt pointer in cobalt_s_dv_timings().
>     The cobalt_info() macro expands to v4l2_info(&cobalt->v4l2_dev, ...),
>     so the function needs a variable of that name, which the previous
>     version was missing. Found by the kernel test robot.
> 
>  drivers/media/pci/cobalt/cobalt-v4l2.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/media/pci/cobalt/cobalt-v4l2.c b/drivers/media/pci/cobalt/cobalt-v4l2.c
> index 51fd9576c..8ffee8ed6 100644
> --- a/drivers/media/pci/cobalt/cobalt-v4l2.c
> +++ b/drivers/media/pci/cobalt/cobalt-v4l2.c
> @@ -617,6 +617,7 @@ static int cobalt_s_dv_timings(struct file *file, void *priv,
>  				    struct v4l2_dv_timings *timings)
>  {
>  	struct cobalt_stream *s = video_drvdata(file);
> +	struct cobalt *cobalt = s->cobalt;
>  	int err;
>  
>  	if (s->input == 1) {
> @@ -630,6 +631,13 @@ static int cobalt_s_dv_timings(struct file *file, void *priv,
>  	if (vb2_is_busy(&s->q))
>  		return -EBUSY;
>  
> +	if (timings->bt.width > COBALT_MAX_WIDTH ||
> +	    timings->bt.height > COBALT_MAX_HEIGHT) {
> +		cobalt_info("timings %ux%u out of range\n",
> +			    timings->bt.width, timings->bt.height);
> +		return -EINVAL;
> +	}
> +
>  	err = v4l2_subdev_call(s->sd,
>  			pad, s_dv_timings, 0, timings);
>  	if (!err) {
> @@ -781,6 +789,14 @@ static int cobalt_try_fmt_vid_cap(struct file *file, void *priv,
>  		break;
>  	}
>  
> +	/*
> +	 * The DMA descriptor buffers are sized for at most
> +	 * COBALT_MAX_WIDTH x COBALT_MAX_HEIGHT, so limit the line stride
> +	 * accordingly.
> +	 */
> +	if (pix->bytesperline > COBALT_MAX_WIDTH * COBALT_MAX_BPP)
> +		pix->bytesperline = COBALT_MAX_WIDTH * COBALT_MAX_BPP;
> +
>  	pix->sizeimage = pix->bytesperline * pix->height;
>  	pix->field = V4L2_FIELD_NONE;
>