diff mbox series

[U-Boot,v4,3/4] SDP: fix wrong usb request size and add high speed endpoint descriptor

Message ID 20190826213905.24881-4-sherry.sun@nxp.com
State Superseded
Headers show
Series Make some changes to SDP | expand

Commit Message

Sherry Sun Aug. 26, 2019, 9:40 a.m. UTC 
Because the buffer length of sdp usb request is 65, we have to allocate
65 bytes not 64 bytes. Otherwise there is potential buffer overflow.

So the wMaxPacketSize of fullspeed can't meet the needs. Add HS
endpoint descriptor for SDP. Then we can use high speed endpoint,
and the SDP device can send packet with 512 byte size.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Ye Li <ye.li@nxp.com>
---
 drivers/usb/gadget/f_sdp.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

Comments

Sherry Sun Sept. 16, 2019, 6:13 a.m. UTC | #1 
Hi Lukasz,

Ping. Any comments on V4?

Best regards
Sherry Sun

> 
> Because the buffer length of sdp usb request is 65, we have to allocate
> 65 bytes not 64 bytes. Otherwise there is potential buffer overflow.
> 
> So the wMaxPacketSize of fullspeed can't meet the needs. Add HS endpoint
> descriptor for SDP. Then we can use high speed endpoint, and the SDP device
> can send packet with 512 byte size.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> Signed-off-by: Ye Li <ye.li@nxp.com>
> ---
>  drivers/usb/gadget/f_sdp.c | 37 ++++++++++++++++++++++++++++++++++---
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c index
> 841814bc07..e7daa1a4f5 100644
> --- a/drivers/usb/gadget/f_sdp.c
> +++ b/drivers/usb/gadget/f_sdp.c
> @@ -70,6 +70,11 @@ struct hid_report {
> 
>  #define SDP_COMMAND_LEN		16
> 
> +/* The first data is for report id, the next 64 bytes are the maximum
> +amount
> + * of data we need to send to usb host. So a total of 65 bytes are needed.
> + */
> +#define SDP_USB_REQUEST_BUFLEN		65
> +
>  struct sdp_command {
>  	u16 cmd;
>  	u32 addr;
> @@ -158,6 +163,16 @@ static struct usb_endpoint_descriptor in_desc = {
>  	.bInterval =		1,
>  };
> 
> +static struct usb_endpoint_descriptor in_hs_desc = {
> +	.bLength =		USB_DT_ENDPOINT_SIZE,
> +	.bDescriptorType =	USB_DT_ENDPOINT,
> /*USB_DT_CS_ENDPOINT*/
> +
> +	.bEndpointAddress =	1 | USB_DIR_IN,
> +	.bmAttributes =	USB_ENDPOINT_XFER_INT,
> +	.wMaxPacketSize =	512,
> +	.bInterval =		1,
> +};
> +
>  static struct usb_descriptor_header *sdp_runtime_descs[] = {
>  	(struct usb_descriptor_header *)&sdp_intf_runtime,
>  	(struct usb_descriptor_header *)&sdp_hid_desc, @@ -165,6 +180,13
> @@ static struct usb_descriptor_header *sdp_runtime_descs[] = {
>  	NULL,
>  };
> 
> +static struct usb_descriptor_header *sdp_runtime_hs_descs[] = {
> +	(struct usb_descriptor_header *)&sdp_intf_runtime,
> +	(struct usb_descriptor_header *)&sdp_hid_desc,
> +	(struct usb_descriptor_header *)&in_hs_desc,
> +	NULL,
> +};
> +
>  /* This is synchronized with what the SoC implementation reports */  static
> struct hid_report sdp_hid_report = {
>  	.usage_page = {
> @@ -490,6 +512,11 @@ static int sdp_bind(struct usb_configuration *c,
> struct usb_function *f)
>  		goto error;
>  	}
> 
> +	if (gadget_is_dualspeed(gadget)) {
> +		/* Assume endpoint addresses are the same for both speeds
> */
> +		in_hs_desc.bEndpointAddress = in_desc.bEndpointAddress;
> +	}
> +
>  	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
> 
>  	cdev->req->context = sdp;
> @@ -527,7 +554,7 @@ static struct usb_request *sdp_start_ep(struct
> usb_ep *ep)  {
>  	struct usb_request *req;
> 
> -	req = alloc_ep_req(ep, 64);
> +	req = alloc_ep_req(ep, SDP_USB_REQUEST_BUFLEN);
>  	debug("%s: ep:%p req:%p\n", __func__, ep, req);
> 
>  	if (!req)
> @@ -542,11 +569,15 @@ static int sdp_set_alt(struct usb_function *f,
> unsigned intf, unsigned alt)  {
>  	struct f_sdp *sdp = func_to_sdp(f);
>  	struct usb_composite_dev *cdev = f->config->cdev;
> +	struct usb_gadget *gadget = cdev->gadget;
>  	int result;
> 
>  	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
> 
> -	result = usb_ep_enable(sdp->in_ep, &in_desc);
> +	if (gadget_is_dualspeed(gadget) && gadget->speed ==
> USB_SPEED_HIGH)
> +		result = usb_ep_enable(sdp->in_ep, &in_hs_desc);
> +	else
> +		result = usb_ep_enable(sdp->in_ep, &in_desc);
>  	if (result)
>  		return result;
>  	sdp->in_req = sdp_start_ep(sdp->in_ep); @@ -592,7 +623,7 @@
> static int sdp_bind_config(struct usb_configuration *c)
>  	memset(sdp_func, 0, sizeof(*sdp_func));
> 
>  	sdp_func->usb_function.name = "sdp";
> -	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
> +	sdp_func->usb_function.hs_descriptors = sdp_runtime_hs_descs;
>  	sdp_func->usb_function.descriptors = sdp_runtime_descs;
>  	sdp_func->usb_function.bind = sdp_bind;
>  	sdp_func->usb_function.unbind = sdp_unbind;
> --
> 2.17.1
diff mbox series

Patch

diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c
index 841814bc07..e7daa1a4f5 100644
--- a/drivers/usb/gadget/f_sdp.c
+++ b/drivers/usb/gadget/f_sdp.c
@@ -70,6 +70,11 @@  struct hid_report {
 
 #define SDP_COMMAND_LEN		16
 
+/* The first data is for report id, the next 64 bytes are the maximum amount
+ * of data we need to send to usb host. So a total of 65 bytes are needed.
+ */
+#define SDP_USB_REQUEST_BUFLEN		65
+
 struct sdp_command {
 	u16 cmd;
 	u32 addr;
@@ -158,6 +163,16 @@  static struct usb_endpoint_descriptor in_desc = {
 	.bInterval =		1,
 };
 
+static struct usb_endpoint_descriptor in_hs_desc = {
+	.bLength =		USB_DT_ENDPOINT_SIZE,
+	.bDescriptorType =	USB_DT_ENDPOINT, /*USB_DT_CS_ENDPOINT*/
+
+	.bEndpointAddress =	1 | USB_DIR_IN,
+	.bmAttributes =	USB_ENDPOINT_XFER_INT,
+	.wMaxPacketSize =	512,
+	.bInterval =		1,
+};
+
 static struct usb_descriptor_header *sdp_runtime_descs[] = {
 	(struct usb_descriptor_header *)&sdp_intf_runtime,
 	(struct usb_descriptor_header *)&sdp_hid_desc,
@@ -165,6 +180,13 @@  static struct usb_descriptor_header *sdp_runtime_descs[] = {
 	NULL,
 };
 
+static struct usb_descriptor_header *sdp_runtime_hs_descs[] = {
+	(struct usb_descriptor_header *)&sdp_intf_runtime,
+	(struct usb_descriptor_header *)&sdp_hid_desc,
+	(struct usb_descriptor_header *)&in_hs_desc,
+	NULL,
+};
+
 /* This is synchronized with what the SoC implementation reports */
 static struct hid_report sdp_hid_report = {
 	.usage_page = {
@@ -490,6 +512,11 @@  static int sdp_bind(struct usb_configuration *c, struct usb_function *f)
 		goto error;
 	}
 
+	if (gadget_is_dualspeed(gadget)) {
+		/* Assume endpoint addresses are the same for both speeds */
+		in_hs_desc.bEndpointAddress = in_desc.bEndpointAddress;
+	}
+
 	sdp->in_ep = ep; /* Store IN EP for enabling @ setup */
 
 	cdev->req->context = sdp;
@@ -527,7 +554,7 @@  static struct usb_request *sdp_start_ep(struct usb_ep *ep)
 {
 	struct usb_request *req;
 
-	req = alloc_ep_req(ep, 64);
+	req = alloc_ep_req(ep, SDP_USB_REQUEST_BUFLEN);
 	debug("%s: ep:%p req:%p\n", __func__, ep, req);
 
 	if (!req)
@@ -542,11 +569,15 @@  static int sdp_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
 	struct f_sdp *sdp = func_to_sdp(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
+	struct usb_gadget *gadget = cdev->gadget;
 	int result;
 
 	debug("%s: intf: %d alt: %d\n", __func__, intf, alt);
 
-	result = usb_ep_enable(sdp->in_ep, &in_desc);
+	if (gadget_is_dualspeed(gadget) && gadget->speed == USB_SPEED_HIGH)
+		result = usb_ep_enable(sdp->in_ep, &in_hs_desc);
+	else
+		result = usb_ep_enable(sdp->in_ep, &in_desc);
 	if (result)
 		return result;
 	sdp->in_req = sdp_start_ep(sdp->in_ep);
@@ -592,7 +623,7 @@  static int sdp_bind_config(struct usb_configuration *c)
 	memset(sdp_func, 0, sizeof(*sdp_func));
 
 	sdp_func->usb_function.name = "sdp";
-	sdp_func->usb_function.hs_descriptors = sdp_runtime_descs;
+	sdp_func->usb_function.hs_descriptors = sdp_runtime_hs_descs;
 	sdp_func->usb_function.descriptors = sdp_runtime_descs;
 	sdp_func->usb_function.bind = sdp_bind;
 	sdp_func->usb_function.unbind = sdp_unbind;