summaryrefslogtreecommitdiffstats
path: root/firmware/usbstack
diff options
context:
space:
mode:
authorAidan MacDonald <amachronic@protonmail.com>2021-09-24 22:41:07 +0100
committerAidan MacDonald <amachronic@protonmail.com>2021-10-16 21:14:42 +0100
commit24294bda15fc1c8c5e838e21f0bac5b5419e5cd2 (patch)
tree633f03121a640f7645a2de6966adc6336e95aeee /firmware/usbstack
parenta665c1faada95e95a51072bfb78d3157ccb5fe85 (diff)
downloadrockbox-24294bda15fc1c8c5e838e21f0bac5b5419e5cd2.tar.gz
rockbox-24294bda15fc1c8c5e838e21f0bac5b5419e5cd2.zip
usb: ensure RX buffers are a multiple of the packet size
When performing an OUT transfer which is not a multiple of the max packet size, the last packet of the OUT transfer should be a short packet. However, there's no guarantee the host sends the expected amount of data in the final packet. The DWC2 USB controller handles this case by accepting any size packet and copying it out to memory. So if the packet is bigger than expected, it'll overrun the caller's buffer and Bad Things will happen. The USB 2.0 spec seems to endorse this behavior. Section 8.5.1 says "an ACK handshake indicates the endpoint has space for a wMaxPacketSize data payload." So it is possible that other USB controllers share the DWC2's behavior. The simplest solution is to force all USB RX buffers to be big enough to hold the transfer size, rounded up to a multiple of the max packet size. For example, a transfer of 700 bytes would require a 1024-byte buffer if the MPS = 512 bytes. Change-Id: Ibb84d2b2d53aec8800a3a7c2449f7a17480acbcf
Diffstat (limited to 'firmware/usbstack')
-rw-r--r--firmware/usbstack/usb_hid.c3
-rw-r--r--firmware/usbstack/usb_serial.c32
-rw-r--r--firmware/usbstack/usb_storage.c2
3 files changed, 25 insertions, 12 deletions
diff --git a/firmware/usbstack/usb_hid.c b/firmware/usbstack/usb_hid.c
index 121736d2dd..64aa123ced 100644
--- a/firmware/usbstack/usb_hid.c
+++ b/firmware/usbstack/usb_hid.c
@@ -666,8 +666,7 @@ void usb_hid_transfer_complete(int ep, int dir, int status, int length)
* to the DAP using the host's custom driver */
static int usb_hid_set_report(struct usb_ctrlrequest *req, void *reqdata)
{
- static unsigned char buf[SET_REPORT_BUF_LEN] USB_DEVBSS_ATTR
- __attribute__((aligned(32)));
+ static unsigned char buf[64] USB_DEVBSS_ATTR __attribute__((aligned(32)));
int length;
if ((req->wValue >> 8) != REPORT_TYPE_OUTPUT)
diff --git a/firmware/usbstack/usb_serial.c b/firmware/usbstack/usb_serial.c
index b49a5ca013..ae90b57078 100644
--- a/firmware/usbstack/usb_serial.c
+++ b/firmware/usbstack/usb_serial.c
@@ -174,7 +174,13 @@ static struct usb_endpoint_descriptor
.bInterval = 0
};
-static struct cdc_line_coding line_coding;
+union line_coding_buffer
+{
+ struct cdc_line_coding data;
+ unsigned char raw[64];
+};
+
+static union line_coding_buffer line_coding USB_DEVBSS_ATTR;
/* send_buffer: local ring buffer.
* transit_buffer: used to store aligned data that will be sent by the USB
@@ -184,10 +190,11 @@ static struct cdc_line_coding line_coding;
*/
#define BUFFER_SIZE 512
#define TRANSIT_BUFFER_SIZE 32
+#define RECV_BUFFER_SIZE 32
static unsigned char send_buffer[BUFFER_SIZE];
static unsigned char transit_buffer[TRANSIT_BUFFER_SIZE]
USB_DEVBSS_ATTR __attribute__((aligned(4)));
-static unsigned char receive_buffer[32]
+static unsigned char receive_buffer[512]
USB_DEVBSS_ATTR __attribute__((aligned(32)));
static void sendout(void);
@@ -293,13 +300,19 @@ bool usb_serial_control_request(struct usb_ctrlrequest* req, void* reqdata, unsi
{
if (req->bRequest == SET_LINE_CODING)
{
- if (req->wLength == sizeof(line_coding))
+ if (req->wLength == sizeof(struct cdc_line_coding))
{
/* Receive line coding into local copy */
- if(!reqdata)
- usb_drv_control_response(USB_CONTROL_RECEIVE, &line_coding, sizeof(line_coding));
+ if (!reqdata)
+ {
+ usb_drv_control_response(USB_CONTROL_RECEIVE, line_coding.raw,
+ sizeof(struct cdc_line_coding));
+ }
else
+ {
usb_drv_control_response(USB_CONTROL_ACK, NULL, 0);
+ }
+
handled = true;
}
}
@@ -317,10 +330,11 @@ bool usb_serial_control_request(struct usb_ctrlrequest* req, void* reqdata, unsi
{
if (req->bRequest == GET_LINE_CODING)
{
- if (req->wLength == sizeof(line_coding))
+ if (req->wLength == sizeof(struct cdc_line_coding))
{
/* Send back line coding so host is happy */
- usb_drv_control_response(USB_CONTROL_ACK, &line_coding, sizeof(line_coding));
+ usb_drv_control_response(USB_CONTROL_ACK, line_coding.raw,
+ sizeof(struct cdc_line_coding));
handled = true;
}
}
@@ -332,7 +346,7 @@ bool usb_serial_control_request(struct usb_ctrlrequest* req, void* reqdata, unsi
void usb_serial_init_connection(void)
{
/* prime rx endpoint */
- usb_drv_recv_nonblocking(ep_out, receive_buffer, sizeof receive_buffer);
+ usb_drv_recv_nonblocking(ep_out, receive_buffer, RECV_BUFFER_SIZE);
/* we come here too after a bus reset, so reset some data */
buffer_transitlength = 0;
@@ -423,7 +437,7 @@ void usb_serial_transfer_complete(int ep,int dir, int status, int length)
/* Data received. TODO : Do something with it ? */
/* Get the next bit */
- usb_drv_recv_nonblocking(ep_out, receive_buffer, sizeof receive_buffer);
+ usb_drv_recv_nonblocking(ep_out, receive_buffer, RECV_BUFFER_SIZE);
break;
case USB_DIR_IN:
diff --git a/firmware/usbstack/usb_storage.c b/firmware/usbstack/usb_storage.c
index 6d79be06ca..a32cf185e7 100644
--- a/firmware/usbstack/usb_storage.c
+++ b/firmware/usbstack/usb_storage.c
@@ -71,7 +71,7 @@
#endif /* USB_READ_BUFFER_SIZE */
/* We don't use sizeof() here, because we *need* a multiple of 32 */
-#define MAX_CBW_SIZE 32
+#define MAX_CBW_SIZE 512
#ifdef USB_WRITE_BUFFER_SIZE
#define WRITE_BUFFER_SIZE USB_WRITE_BUFFER_SIZE