* [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out
@ 2026-05-13 8:52 Hans Verkuil
2026-05-13 12:49 ` Hans Verkuil
2026-05-13 14:59 ` Nicolas Dufresne
0 siblings, 2 replies; 3+ messages in thread
From: Hans Verkuil @ 2026-05-13 8:52 UTC (permalink / raw)
To: Linux Media Mailing List
The vivid_update_format_cap/out() functions must only be called if the
capture/output queue are not busy. But for several controls that is not
checked.
Only when streaming starts will they be set to 'grabbed' and it is
impossible to change the control, but between REQBUFS and STREAMON you
are still allowed to set these controls. Since vivid_update_format_cap/out
will change the format, this can cause unexpected results.
I suspect that this is the cause of this syzbot bug:
https://syzkaller.appspot.com/bug?extid=dac8f5eaa46837e97b89
But since we never have reproducers, it is hard to be certain. In any case,
these checks are needed regardless.
Reported-by: syzbot+dac8f5eaa46837e97b89@syzkaller.appspotmail.com
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index f94c15ff84f7..e40ff999cad8 100644
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -608,18 +608,26 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
tpg_s_vflip(&dev->tpg, dev->sensor_vflip ^ dev->vflip);
break;
case VIVID_CID_REDUCED_FPS:
+ if (vb2_is_busy(&dev->vb_vid_cap_q))
+ return -EBUSY;
dev->reduced_fps = ctrl->val;
vivid_update_format_cap(dev, true);
break;
case VIVID_CID_HAS_CROP_CAP:
+ if (vb2_is_busy(&dev->vb_vid_cap_q))
+ return -EBUSY;
dev->has_crop_cap = ctrl->val;
vivid_update_format_cap(dev, true);
break;
case VIVID_CID_HAS_COMPOSE_CAP:
+ if (vb2_is_busy(&dev->vb_vid_cap_q))
+ return -EBUSY;
dev->has_compose_cap = ctrl->val;
vivid_update_format_cap(dev, true);
break;
case VIVID_CID_HAS_SCALER_CAP:
+ if (vb2_is_busy(&dev->vb_vid_cap_q))
+ return -EBUSY;
dev->has_scaler_cap = ctrl->val;
vivid_update_format_cap(dev, true);
break;
@@ -1116,14 +1124,20 @@ static int vivid_vid_out_s_ctrl(struct v4l2_ctrl *ctrl)
switch (ctrl->id) {
case VIVID_CID_HAS_CROP_OUT:
+ if (vb2_is_busy(&dev->vb_vid_out_q))
+ return -EBUSY;
dev->has_crop_out = ctrl->val;
vivid_update_format_out(dev);
break;
case VIVID_CID_HAS_COMPOSE_OUT:
+ if (vb2_is_busy(&dev->vb_vid_out_q))
+ return -EBUSY;
dev->has_compose_out = ctrl->val;
vivid_update_format_out(dev);
break;
case VIVID_CID_HAS_SCALER_OUT:
+ if (vb2_is_busy(&dev->vb_vid_out_q))
+ return -EBUSY;
dev->has_scaler_out = ctrl->val;
vivid_update_format_out(dev);
break;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out
2026-05-13 8:52 [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out Hans Verkuil
@ 2026-05-13 12:49 ` Hans Verkuil
2026-05-13 14:59 ` Nicolas Dufresne
1 sibling, 0 replies; 3+ messages in thread
From: Hans Verkuil @ 2026-05-13 12:49 UTC (permalink / raw)
To: Linux Media Mailing List
On 13/05/2026 10:52, Hans Verkuil wrote:
> The vivid_update_format_cap/out() functions must only be called if the
> capture/output queue are not busy. But for several controls that is not
> checked.
>
> Only when streaming starts will they be set to 'grabbed' and it is
> impossible to change the control, but between REQBUFS and STREAMON you
> are still allowed to set these controls. Since vivid_update_format_cap/out
> will change the format, this can cause unexpected results.
>
> I suspect that this is the cause of this syzbot bug:
>
> https://syzkaller.appspot.com/bug?extid=dac8f5eaa46837e97b89
>
> But since we never have reproducers, it is hard to be certain. In any case,
> these checks are needed regardless.
>
> Reported-by: syzbot+dac8f5eaa46837e97b89@syzkaller.appspotmail.com
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Oops, this should be Hans Verkuil <hverkuil+cisco@kernel.org>. Also add:
Fixes: c79aa6aeadb0 ("[media] vivid-capture: add control for reduced frame rate")
Cc: stable@vger.kernel.org
Regards,
Hans
> ---
> diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
> index f94c15ff84f7..e40ff999cad8 100644
> --- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
> +++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
> @@ -608,18 +608,26 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
> tpg_s_vflip(&dev->tpg, dev->sensor_vflip ^ dev->vflip);
> break;
> case VIVID_CID_REDUCED_FPS:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->reduced_fps = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_CROP_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_crop_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_COMPOSE_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_compose_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_SCALER_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_scaler_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> @@ -1116,14 +1124,20 @@ static int vivid_vid_out_s_ctrl(struct v4l2_ctrl *ctrl)
>
> switch (ctrl->id) {
> case VIVID_CID_HAS_CROP_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_crop_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
> case VIVID_CID_HAS_COMPOSE_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_compose_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
> case VIVID_CID_HAS_SCALER_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_scaler_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out
2026-05-13 8:52 [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out Hans Verkuil
2026-05-13 12:49 ` Hans Verkuil
@ 2026-05-13 14:59 ` Nicolas Dufresne
1 sibling, 0 replies; 3+ messages in thread
From: Nicolas Dufresne @ 2026-05-13 14:59 UTC (permalink / raw)
To: Hans Verkuil, Linux Media Mailing List
[-- Attachment #1: Type: text/plain, Size: 3052 bytes --]
Le mercredi 13 mai 2026 à 10:52 +0200, Hans Verkuil a écrit :
> The vivid_update_format_cap/out() functions must only be called if the
> capture/output queue are not busy. But for several controls that is not
> checked.
>
> Only when streaming starts will they be set to 'grabbed' and it is
> impossible to change the control, but between REQBUFS and STREAMON you
> are still allowed to set these controls. Since vivid_update_format_cap/out
> will change the format, this can cause unexpected results.
>
> I suspect that this is the cause of this syzbot bug:
>
> https://syzkaller.appspot.com/bug?extid=dac8f5eaa46837e97b89
>
> But since we never have reproducers, it is hard to be certain. In any case,
> these checks are needed regardless.
>
> Reported-by: syzbot+dac8f5eaa46837e97b89@syzkaller.appspotmail.com
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
> diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c
> b/drivers/media/test-drivers/vivid/vivid-ctrls.c
> index f94c15ff84f7..e40ff999cad8 100644
> --- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
> +++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
> @@ -608,18 +608,26 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
> tpg_s_vflip(&dev->tpg, dev->sensor_vflip ^ dev->vflip);
> break;
> case VIVID_CID_REDUCED_FPS:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
It is unclear to me why users need to free all the buffers to enable this
feature. Should that one only be limited to not being streaming ? I believe it
will only affect the HDMI DV Timings and the frame internal in params right ?
> dev->reduced_fps = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_CROP_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_crop_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_COMPOSE_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_compose_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> case VIVID_CID_HAS_SCALER_CAP:
> + if (vb2_is_busy(&dev->vb_vid_cap_q))
> + return -EBUSY;
> dev->has_scaler_cap = ctrl->val;
> vivid_update_format_cap(dev, true);
> break;
> @@ -1116,14 +1124,20 @@ static int vivid_vid_out_s_ctrl(struct v4l2_ctrl
> *ctrl)
>
> switch (ctrl->id) {
> case VIVID_CID_HAS_CROP_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_crop_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
> case VIVID_CID_HAS_COMPOSE_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_compose_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
> case VIVID_CID_HAS_SCALER_OUT:
> + if (vb2_is_busy(&dev->vb_vid_out_q))
> + return -EBUSY;
> dev->has_scaler_out = ctrl->val;
> vivid_update_format_out(dev);
> break;
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-13 14:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-13 8:52 [PATCH] media: vivid: check vb2_is_busy before calling vivid_update_format_cap/out Hans Verkuil
2026-05-13 12:49 ` Hans Verkuil
2026-05-13 14:59 ` Nicolas Dufresne
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox