All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Grzeschik <m.grzeschik@pengutronix.de>
To: linux-usb@vger.kernel.org
Cc: linux-media@vger.kernel.org, gregkh@linuxfoundation.org,
	balbi@kernel.org, laurent.pinchart@ideasonboard.com,
	kernel@pengutronix.de
Subject: [PATCH v8] usb: gadget: uvc: add validate and fix function for uvc response
Date: Wed, 30 Nov 2022 14:28:55 +0100	[thread overview]
Message-ID: <20221130132855.1263114-1-m.grzeschik@pengutronix.de> (raw)

When the userspace gets the setup requests for UVC_GET_CUR, UVC_GET_MIN,
UVC_GET_MAX, UVC_GET_DEF it will fill out the ctrl response. This data
needs to be validated. Since the kernel also knows the limits we can
return EINVAL if the userspace does try to send invalid data to the
host.

For UVC_GET_MAX and UVC_GET_MIN we fixup the bFrameIndex and
bFormatIndex to the correct boundaries. And for all other requests we
set valid dwFrameInterval, dwMaxPayloadTransferSize and
dwMaxVideoFrameSize if the userspace did not set any value at all.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1: -> v4:
- new patch
v4: -> v5:
- changed uvcg_info to uvcg_dbg for fixups, updated info strings
v5: -> v6:
- no changes
v6 -> v7:
- reworked to not need 'd182bf156c4c ("usb: gadget: uvc: default the ctrl request interface offsets")'
v7 -> v8:
- splitup validate and fix to separate functions
- only validate on UVC_GET_CUR
- return with EINVAL if validate fails
- fixup on UVC_GET_MIN, UVC_GET_MAX and UVC_GET_DEF
- rebased again on necessary patch 'd182bf156c4c ("usb: gadget: uvc: default the ctrl request interface offsets")'
- after validating one streaming request unset the current streaming_request

 drivers/usb/gadget/function/f_uvc.c    |   4 +-
 drivers/usb/gadget/function/uvc.h      |   1 +
 drivers/usb/gadget/function/uvc_v4l2.c | 154 +++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e131624011a5e..098bd3c4e3c0b3 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -254,8 +254,10 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	 */
 	mctrl = &uvc_event->req;
 	mctrl->wIndex &= ~cpu_to_le16(0xff);
-	if (interface == uvc->streaming_intf)
+	if (interface == uvc->streaming_intf) {
+		uvc->streaming_request = ctrl->bRequest;
 		mctrl->wIndex = cpu_to_le16(UVC_STRING_STREAMING_IDX);
+	}
 
 	v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e148a..1be4d5f24b46bf 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -151,6 +151,7 @@ struct uvc_device {
 	void *control_buf;
 
 	unsigned int streaming_intf;
+	unsigned char streaming_request;
 
 	/* Events */
 	unsigned int event_length;
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a189b08bba800d..a5a109e7708f4b 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -178,6 +178,137 @@ static struct uvcg_frame *find_closest_frame_by_size(struct uvc_device *uvc,
  * Requests handling
  */
 
+/* validate streaming ctrl request response data for valid parameters */
+static int
+uvc_validate_streaming_ctrl(struct uvc_device *uvc,
+			    struct uvc_streaming_control *ctrl)
+{
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
+	unsigned int iformat, iframe;
+	struct uvcg_format *uformat;
+	struct uvcg_frame *uframe;
+	bool ival_found = false;
+	int i;
+
+	iformat = ctrl->bFormatIndex;
+	iframe = ctrl->bFrameIndex;
+
+	iformat = clamp((unsigned int)iformat, 1U,
+			(unsigned int)uvc->header->num_fmt);
+	if (iformat != ctrl->bFormatIndex) {
+		uvcg_dbg(&uvc->func,
+			 "invalid bFormatIndex set: %d\n",
+			 ctrl->bFormatIndex);
+		return -EINVAL;
+	}
+	uformat = find_format_by_index(uvc, iformat);
+
+	iframe = clamp((unsigned int)iframe, 1U,
+		       (unsigned int)uformat->num_frames);
+	if (iframe != ctrl->bFrameIndex) {
+		uvcg_dbg(&uvc->func,
+			 "invalid bFrameIndex set: %d\n",
+			 ctrl->bFrameIndex);
+		return -EINVAL;
+	}
+	uframe = find_frame_by_index(uvc, uformat, iframe);
+
+	if (ctrl->dwFrameInterval) {
+		for (i = 0; i < uframe->frame.b_frame_interval_type; i++) {
+			if (ctrl->dwFrameInterval ==
+				 uframe->dw_frame_interval[i]) {
+				ival_found = true;
+				break;
+			}
+		}
+		if (!ival_found) {
+			uvcg_dbg(&uvc->func,
+				 "invalid dwFrameInterval set: %d\n",
+				 ctrl->dwFrameInterval);
+			return -EINVAL;
+		}
+	}
+
+	if (ctrl->dwMaxPayloadTransferSize >
+				opts->streaming_maxpacket) {
+		uvcg_dbg(&uvc->func,
+			  "invalid dwMaxPayloadTransferSize set: %d\n",
+			  ctrl->dwMaxPayloadTransferSize);
+		return -EINVAL;
+	}
+
+	if (ctrl->dwMaxVideoFrameSize >
+				uframe->frame.dw_max_video_frame_buffer_size) {
+		uvcg_dbg(&uvc->func,
+			  "invalid dwMaxVideoFrameSize set: %d\n",
+			  ctrl->dwMaxVideoFrameSize);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void
+uvc_fixup_streaming_ctrl(struct uvc_device *uvc,
+			    struct uvc_streaming_control *ctrl)
+{
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
+	unsigned int iformat, iframe;
+	struct uvcg_format *uformat;
+	struct uvcg_frame *uframe;
+
+	iformat = ctrl->bFormatIndex;
+	iframe = ctrl->bFrameIndex;
+
+	/* Fixup the bFormatIndex on UVC_GET_{MIN,MAX} events */
+	if (ctrl->bFormatIndex == U8_MAX || !ctrl->bFormatIndex) {
+		iformat = clamp((unsigned int)iformat, 1U,
+				(unsigned int)uvc->header->num_fmt);
+		ctrl->bFormatIndex = iformat;
+		uvcg_dbg(&uvc->func,
+			  "fixup bFormatIndex: %d\n",
+			  ctrl->bFormatIndex);
+	}
+
+	uformat = find_format_by_index(uvc, iformat);
+
+	/* Fixup the bFrameIndex on UVC_GET_{MIN,MAX} events */
+	if (ctrl->bFrameIndex == U8_MAX || !ctrl->bFrameIndex) {
+		iframe = clamp((unsigned int)iframe, 1U,
+			       (unsigned int)uformat->num_frames);
+		ctrl->bFrameIndex = iframe;
+		uvcg_dbg(&uvc->func,
+			  "fixup bFrameIndex: %d\n",
+			  ctrl->bFrameIndex);
+	}
+
+	uframe = find_frame_by_index(uvc, uformat, iframe);
+
+	/* Fixup values where the userspace does not set the paramters
+	 * but does handover the preparing of the values to the kernel.
+	 */
+	if (!ctrl->dwFrameInterval) {
+		ctrl->dwFrameInterval = uframe->frame.dw_default_frame_interval;
+		uvcg_dbg(&uvc->func,
+			  "fixup dwFrameInterval: %d\n",
+			  ctrl->dwFrameInterval);
+	}
+
+	if (!ctrl->dwMaxPayloadTransferSize) {
+		ctrl->dwMaxPayloadTransferSize = opts->streaming_maxpacket;
+		uvcg_dbg(&uvc->func,
+			  "fixup dwMaxPayloadTransferSize: %d\n",
+			  ctrl->dwMaxPayloadTransferSize);
+	}
+
+	if (!ctrl->dwMaxVideoFrameSize) {
+		ctrl->dwMaxVideoFrameSize = uvc_get_frame_size(uformat, uframe);
+		uvcg_dbg(&uvc->func,
+			  "fixup dwMaxVideoFrameSize: %d\n",
+			  ctrl->dwMaxVideoFrameSize);
+	}
+}
+
 static int
 uvc_send_response(struct uvc_device *uvc, struct uvc_request_data *data)
 {
@@ -192,6 +323,29 @@ uvc_send_response(struct uvc_device *uvc, struct uvc_request_data *data)
 
 	memcpy(req->buf, data->data, req->length);
 
+	/* validate the ctrl content and fixup */
+	if (!uvc->event_setup_out) {
+		struct uvc_streaming_control *ctrl = req->buf;
+		int ret;
+
+		switch (uvc->streaming_request) {
+		case UVC_GET_CUR:
+			ret = uvc_validate_streaming_ctrl(uvc, ctrl);
+			if (ret)
+				return ret;
+			fallthrough;
+		case UVC_GET_MIN:
+		case UVC_GET_MAX:
+		case UVC_GET_DEF:
+			uvc_fixup_streaming_ctrl(uvc, ctrl);
+			break;
+		default:
+			break;
+		}
+
+		uvc->streaming_request = UVC_RC_UNDEFINED;
+	}
+
 	return usb_ep_queue(cdev->gadget->ep0, req, GFP_KERNEL);
 }
 
-- 
2.30.2


             reply	other threads:[~2022-11-30 13:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-30 13:28 Michael Grzeschik [this message]
2022-12-03 21:27 ` [PATCH v8] usb: gadget: uvc: add validate and fix function for uvc response Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221130132855.1263114-1-m.grzeschik@pengutronix.de \
    --to=m.grzeschik@pengutronix.de \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel@pengutronix.de \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.