[PATCH v3 1/4] usb: gadget: uvc: synchronize streamon/off with uvc_function_set_alt
From: Paul Elder
Date: Wed Jan 09 2019 - 02:11:30 EST
If the streamon ioctl is issued while the stream is already on, then the
kernel BUGs. This happens at the BUG_ON in uvc_video_alloc_requests
within the call stack from the ioctl handler for VIDIOC_STREAMON.
This could happen when uvc_function_set_alt 0 races and wins against
uvc_v4l2_streamon, or when userspace neglects to issue the
VIDIOC_STREAMOFF ioctl.
To fix this, add two more uvc states: starting and stopping. Use these
to prevent the racing, and to detect when VIDIOC_STREAMON is issued
without previously issuing VIDIOC_STREAMOFF.
Signed-off-by: Paul Elder <paul.elder@xxxxxxxxxxxxxxxx>
---
Changes in v3:
- add state guard to uvc_function_set_alt 1
- add documentation for newly added uvc states
- reorder uvc states to more or less follow the flow diagram
- add more state guards to ioctl handlers for streamon and streamoff
Changes in v2: Nothing
drivers/usb/gadget/function/f_uvc.c | 17 ++++++++----
drivers/usb/gadget/function/uvc.h | 37 ++++++++++++++++++++++++++
drivers/usb/gadget/function/uvc_v4l2.c | 26 ++++++++++++++++--
3 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 8c99392df593..2ec3b73b2b75 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -317,26 +317,31 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
switch (alt) {
case 0:
- if (uvc->state != UVC_STATE_STREAMING)
+ if (uvc->state != UVC_STATE_STREAMING &&
+ uvc->state != UVC_STATE_STARTING)
return 0;
if (uvc->video.ep)
usb_ep_disable(uvc->video.ep);
+ uvc->state = UVC_STATE_STOPPING;
+
memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_STREAMOFF;
v4l2_event_queue(&uvc->vdev, &v4l2_event);
- uvc->state = UVC_STATE_CONNECTED;
return 0;
case 1:
- if (uvc->state != UVC_STATE_CONNECTED)
- return 0;
-
if (!uvc->video.ep)
return -EINVAL;
+ if (uvc->state == UVC_STATE_STOPPING)
+ return -EINVAL;
+
+ if (uvc->state != UVC_STATE_CONNECTED)
+ return 0;
+
uvcg_info(f, "reset UVC\n");
usb_ep_disable(uvc->video.ep);
@@ -346,6 +351,8 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
return ret;
usb_ep_enable(uvc->video.ep);
+ uvc->state = UVC_STATE_STARTING;
+
memset(&v4l2_event, 0, sizeof(v4l2_event));
v4l2_event.type = UVC_EVENT_STREAMON;
v4l2_event_queue(&uvc->vdev, &v4l2_event);
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 099d650082e5..f183e349499c 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -101,10 +101,47 @@ struct uvc_video {
unsigned int fid;
};
+/**
+ * enum uvc_state - the states of a struct uvc_device
+ * @UVC_STATE_DISCONNECTED: not connected state
+ * - transition to connected state on .set_alt
+ * @UVC_STATE_CONNECTED: connected state
+ * - transition to disconnected state on .disable
+ * and alloc
+ * - transition to starting state on .set_alt 1
+ * @UVC_STATE_STARTING: starting state
+ * - transition to streaming state on streamon ioctl
+ * - transition to stopping state on set_alt 0
+ * @UVC_STATE_STREAMING: streaming state
+ * - transition to stopping state on .set_alt 0
+ * @UVC_STATE_STOPPING: stopping state
+ * - transition to connected on streamoff ioctl
+ *
+ * Diagram of state transitions:
+ *
+ * disable
+ * +---------------------------+
+ * v |
+ * +--------------+ set_alt +-----------+
+ * | DISCONNECTED | ---------> | CONNECTED |
+ * +--------------+ +-----------+
+ * | ^
+ * set_alt 1 | | streamoff
+ * +----------------------+ --------------------+
+ * V |
+ * +----------+ streamon +-----------+ set_alt 0 +----------+
+ * | STARTING | ----------> | STREAMING | -----------> | STOPPING |
+ * +----------+ +-----------+ +----------+
+ * | ^
+ * | set_alt 0 |
+ * +------------------------------------------------+
+ */
enum uvc_state {
UVC_STATE_DISCONNECTED,
UVC_STATE_CONNECTED,
+ UVC_STATE_STARTING,
UVC_STATE_STREAMING,
+ UVC_STATE_STOPPING,
};
struct uvc_device {
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a1183eccee22..97e624214287 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -197,17 +197,24 @@ uvc_v4l2_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
if (type != video->queue.queue.type)
return -EINVAL;
+ if (uvc->state == UVC_STATE_STREAMING)
+ return 0;
+
+ if (uvc->state != UVC_STATE_STARTING)
+ return -EINVAL;
+
/* Enable UVC video. */
ret = uvcg_video_enable(video, 1);
if (ret < 0)
return ret;
+ uvc->state = UVC_STATE_STREAMING;
+
/*
* Complete the alternate setting selection setup phase now that
* userspace is ready to provide video frames.
*/
uvc_function_setup_continue(uvc);
- uvc->state = UVC_STATE_STREAMING;
return 0;
}
@@ -218,11 +225,26 @@ uvc_v4l2_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
struct video_device *vdev = video_devdata(file);
struct uvc_device *uvc = video_get_drvdata(vdev);
struct uvc_video *video = &uvc->video;
+ int ret;
if (type != video->queue.queue.type)
return -EINVAL;
- return uvcg_video_enable(video, 0);
+ /*
+ * Check for connected state also because we want to reset buffers
+ * if this is called when the stream is already off.
+ */
+ if (uvc->state != UVC_STATE_STOPPING &&
+ uvc->state != UVC_STATE_CONNECTED)
+ return 0;
+
+ ret = uvcg_video_enable(video, 0);
+ if (ret < 0)
+ return ret;
+
+ uvc->state = UVC_STATE_CONNECTED;
+
+ return 0;
}
static int
--
2.20.1