media: vivid: check for vb2_is_busy() when toggling caps

The vivid_update_format_cap/out() functions must only be called if the
capture/output queue are not busy. But for the controls that select
the CROP/COMPOSE/SCALE capability 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.

Besides adding these checks, also add a WARN_ON in
vivid_update_format_cap/out() if the queue is busy.

I'm 90% certain 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.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Fixes: 73c3f48230cd ("[media] vivid: add the control handling code")
Cc: stable@vger.kernel.org
Reported-by: syzbot+dac8f5eaa46837e97b89@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=dac8f5eaa46837e97b89
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c
index 1077445f577260a87dbb9fc9d7446bb57ef93fc5..a8a134b36720eb56683fa6b3175032a090db1a2d 100644 (file)
--- a/drivers/media/test-drivers/vivid/vivid-ctrls.c
+++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c
@@ -613,14 +613,20 @@ static int vivid_vid_cap_s_ctrl(struct v4l2_ctrl *ctrl)
                        vivid_update_reduced_fps(dev);
                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;
@@ -1117,14 +1123,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;

diff --git a/drivers/media/test-drivers/vivid/vivid-vid-cap.c b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
index 76e0b161c049af7ba40d36c589a486e427c31b53..e204490847095c69f44635b5e9e99c1bc9fc75ec 100644 (file)
--- a/drivers/media/test-drivers/vivid/vivid-vid-cap.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-cap.c
@@ -391,6 +391,12 @@ void vivid_update_format_cap(struct vivid_dev *dev, bool keep_controls)
        struct v4l2_bt_timings *bt = &dev->dv_timings_cap[dev->input].bt;
        u32 dims[V4L2_CTRL_MAX_DIMS] = {};
 
+       /*
+        * This resets the format, so must never be called while vb2_is_busy().
+        */
+       if (WARN_ON(vb2_is_busy(&dev->vb_vid_cap_q)))
+               return;
+
        switch (dev->input_type[dev->input]) {
        case WEBCAM:
        default:

diff --git a/drivers/media/test-drivers/vivid/vivid-vid-out.c b/drivers/media/test-drivers/vivid/vivid-vid-out.c
index 8c037b90833e74563654a60f8c8ecc292e21151c..23e1d5a189eed134d8ced63cd99df8f66642c02a 100644 (file)
--- a/drivers/media/test-drivers/vivid/vivid-vid-out.c
+++ b/drivers/media/test-drivers/vivid/vivid-vid-out.c
@@ -214,6 +214,12 @@ void vivid_update_format_out(struct vivid_dev *dev)
        unsigned size, p;
        u64 pixelclock;
 
+       /*
+        * This resets the format, so must never be called while vb2_is_busy().
+        */
+       if (WARN_ON(vb2_is_busy(&dev->vb_vid_out_q)))
+               return;
+
        switch (dev->output_type[dev->output]) {
        case SVID:
        default: