Tests: USB: Check ep buffer is released by abort

Filip Jagodzinski · c1728p9 · commit 6086e1fa6536 · 2018-10-03T14:52:40.000-05:00
Validate that endpoint buffer is not used after a transfer has been
aborted.
diff --git a/TESTS/host_tests/pyusb_basic.py b/TESTS/host_tests/pyusb_basic.py
@@ -56,6 +56,7 @@ def get_interface(dev, interface, alternate=0):
 VENDOR_TEST_CTRL_IN_SIZES = 9
 VENDOR_TEST_CTRL_OUT_SIZES = 10
 VENDOR_TEST_READ_START = 11
+VENDOR_TEST_ABORT_BUFF_CHECK = 12
 VENDOR_TEST_UNSUPPORTED_REQUEST = 32
 
 REQUEST_GET_STATUS = 0
@@ -1062,6 +1063,16 @@ def request_endpoint_read_start(dev, ep):
     dev.ctrl_transfer(**ctrl_kwargs)
 
 
+def request_abort_buff_check(dev, ep):
+    ctrl_kwargs = {
+        'bmRequestType': build_request_type(CTRL_IN, CTRL_TYPE_VENDOR, CTRL_RECIPIENT_ENDPOINT),
+        'bRequest': VENDOR_TEST_ABORT_BUFF_CHECK,
+        'wValue': 0,
+        'wIndex': ep.bEndpointAddress,
+        'data_or_wLength': 1}
+    return bool(dev.ctrl_transfer(**ctrl_kwargs)[0])
+
+
 USB_ERROR_FMT = str('Got {0!r} while testing endpoints '
                     '{1.bEndpointAddress:#04x}({1.wMaxPacketSize:02}) and '
                     '{2.bEndpointAddress:#04x}({2.wMaxPacketSize:02}) with '
@@ -1304,9 +1315,16 @@ def ep_test_abort(dev, log, verbose=False):
     Given a USB device with multiple OUT/IN endpoint pairs
     When a device aborts an in progress data transfer
     Then no more data is transmitted
+        and endpoint buffer is correctly released on the device end
     """
     NUM_PACKETS_UNTIL_ABORT = 2
     NUM_PACKETS_AFTER_ABORT = 8
+
+    # If the host ever receives a payload with any byte set to this value,
+    # the device does not handle abort operation correctly. The buffer
+    # passed to aborted operation must not be used after call to abort().
+    FORBIDDEN_PAYLOAD_VALUE = NUM_PACKETS_AFTER_ABORT + 1
+
     cfg = dev.get_active_configuration()
     for intf in cfg:
         log('interface {}, alt {} -- '.format(intf.bInterfaceNumber, intf.bAlternateSetting), end='')
@@ -1336,6 +1354,11 @@ def ep_test_abort(dev, log, verbose=False):
                     payload_in.extend(packet)
                 except usb.core.USBError as err:
                     break
+            if FORBIDDEN_PAYLOAD_VALUE in payload_in:
+                raise_unconditionally(
+                    lineno(), 'Endpoint buffer not released when aborting the '
+                    'write operation on endpoint {0.bEndpointAddress:#04x}.'
+                    .format(ep_in))
             if verbose:
                 log('The size of data successfully received from endpoint {0.bEndpointAddress:#04x}: {1} B.'
                     .format(ep_in, len(payload_in)))
@@ -1353,13 +1376,24 @@ def ep_test_abort(dev, log, verbose=False):
             log('Testing aborting an in progress transfer for OUT endpoints.')
         for ep_out in (bulk_out, interrupt_out):
             payload_size = (NUM_PACKETS_UNTIL_ABORT + NUM_PACKETS_AFTER_ABORT) * ep_out.wMaxPacketSize
-            payload_out = array.array('B', (0x01 for _ in range(ep_out.wMaxPacketSize)))
             num_bytes_written = 0
             while num_bytes_written < payload_size:
+                payload_out = array.array('B', (num_bytes_written/ep_out.wMaxPacketSize
+                                                for _ in range(ep_out.wMaxPacketSize)))
                 try:
                     num_bytes_written += ep_out.write(payload_out)
                 except usb.core.USBError:
                     break
+            try:
+                ep_buff_correct = request_abort_buff_check(dev, ep_out)
+            except (usb.core.USBError, IndexError, TypeError) as err:
+                raise_unconditionally(
+                    lineno(), 'Unable to verify endpoint buffer content ({!r}).'.format(err))
+            if not ep_buff_correct:
+                raise_unconditionally(
+                    lineno(), 'Endpoint buffer not released when aborting the '
+                    'read operation on endpoint {0.bEndpointAddress:#04x}.'
+                    .format(ep_out))
             if verbose:
                 log('The size of data successfully sent to endpoint {0.bEndpointAddress:#04x}: {1} B.'
                     .format(ep_out, num_bytes_written))
diff --git a/TESTS/usb_device/basic/USBEndpointTester.cpp b/TESTS/usb_device/basic/USBEndpointTester.cpp
@@ -24,12 +24,20 @@
 
 #define NUM_PACKETS_UNTIL_ABORT     2
 #define NUM_PACKETS_AFTER_ABORT     8
+#define EP_ABORT_BUFF_VALUE         0xff
 
-#define VENDOR_TEST_CTRL_IN         1
-#define VENDOR_TEST_CTRL_OUT        2
-#define VENDOR_TEST_CTRL_IN_SIZES   9
-#define VENDOR_TEST_CTRL_OUT_SIZES  10
-#define VENDOR_TEST_READ_START      11
+/* If the host ever receives a payload with any byte set to this value,
+ * the device does not handle abort operation correctly. The buffer
+ * passed to aborted operation must not be used after call to abort().
+ */
+#define FORBIDDEN_PAYLOAD_VALUE     (NUM_PACKETS_AFTER_ABORT + 1)
+
+#define VENDOR_TEST_CTRL_IN             1
+#define VENDOR_TEST_CTRL_OUT            2
+#define VENDOR_TEST_CTRL_IN_SIZES       9
+#define VENDOR_TEST_CTRL_OUT_SIZES      10
+#define VENDOR_TEST_READ_START          11
+#define VENDOR_TEST_ABORT_BUFF_CHECK    12
 
 #define EVENT_READY (1 << 0)
 
@@ -236,6 +244,12 @@ void USBEndpointTester::callback_request(const setup_packet_t *setup)
             case VENDOR_TEST_READ_START:
                 result = (_request_read_start(setup)) ? Success : Failure;
                 break;
+            case VENDOR_TEST_ABORT_BUFF_CHECK:
+                result = Send;
+                ctrl_buf[0] = _request_abort_buff_check(setup);
+                data = ctrl_buf;
+                size = 1;
+                break;
             default:
                 result = PassThrough;
                 break;
@@ -273,20 +287,50 @@ bool USBEndpointTester::_request_read_start(const setup_packet_t *setup)
     if (setup->bmRequestType.Recipient != ENDPOINT_RECIPIENT) {
         return false;
     }
-    size_t ep_index = NUM_ENDPOINTS + 1;
+    size_t ep_index = NUM_ENDPOINTS;
     for (size_t i = 0; i < NUM_ENDPOINTS; i++) {
         if (_endpoints[i] == setup->wIndex) {
             ep_index = i;
             break;
         }
     }
-    if (ep_index > NUM_ENDPOINTS) {
+    if (ep_index == NUM_ENDPOINTS) {
+        return false;
+    }
+    if (_endpoint_buffs[ep_index] == NULL) {
         return false;
     }
     endpoint_abort(_endpoints[ep_index]);
     return read_start(_endpoints[ep_index], _endpoint_buffs[ep_index], (*_endpoint_configs)[ep_index].max_packet);
 }
 
+bool USBEndpointTester::_request_abort_buff_check(const setup_packet_t *setup)
+{
+    assert_locked();
+    if (setup->bmRequestType.Recipient != ENDPOINT_RECIPIENT) {
+        return false;
+    }
+    size_t ep_index = NUM_ENDPOINTS;
+    for (size_t i = 0; i < NUM_ENDPOINTS; i++) {
+        if (_endpoints[i] == setup->wIndex) {
+            ep_index = i;
+            break;
+        }
+    }
+    if (ep_index == NUM_ENDPOINTS) {
+        return false;
+    }
+    if (_endpoint_buffs[ep_index] == NULL) {
+        return false;
+    }
+    for (size_t i = 0; i < (*_endpoint_configs)[ep_index].max_packet; i++) {
+        if (_endpoint_buffs[ep_index][i] != EP_ABORT_BUFF_VALUE) {
+            return false;
+        }
+    }
+    return true;
+}
+
 void USBEndpointTester::callback_request_xfer_done(const setup_packet_t *setup, bool aborted)
 {
     if (aborted) {
@@ -309,6 +353,9 @@ void USBEndpointTester::callback_request_xfer_done(const setup_packet_t *setup,
             case VENDOR_TEST_CTRL_IN_SIZES:
                 result = true;
                 break;
+            case VENDOR_TEST_ABORT_BUFF_CHECK:
+                result = true;
+                break;
             default:
                 result = false;
                 break;
@@ -723,7 +770,6 @@ void USBEndpointTester::_cb_bulk_out()
 {
     _cnt_cb_bulk_out++;
     uint32_t rx_size = read_finish(_endpoints[EP_BULK_OUT]);
-
     if (_abort_transfer_test == false) {
         // Send data back to host using the IN endpoint.
         memset(_endpoint_buffs[EP_BULK_IN], 0, (*_endpoint_configs)[EP_BULK_IN].max_packet);
@@ -732,6 +778,10 @@ void USBEndpointTester::_cb_bulk_out()
     } else {
         // Abort the transfer if enough data was received.
         _num_packets_bulk_out_abort++;
+        if (_num_packets_bulk_out_abort == NUM_PACKETS_UNTIL_ABORT) {
+            // Set every byte of the buffer to a known value.
+            memset(_endpoint_buffs[EP_BULK_OUT], EP_ABORT_BUFF_VALUE, (*_endpoint_configs)[EP_BULK_OUT].max_packet);
+        }
         read_start(_endpoints[EP_BULK_OUT], _endpoint_buffs[EP_BULK_OUT], (*_endpoint_configs)[EP_BULK_OUT].max_packet);
         if (_num_packets_bulk_out_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(_endpoints[EP_BULK_OUT]);
@@ -743,7 +793,6 @@ void USBEndpointTester::_cb_bulk_in()
 {
     _cnt_cb_bulk_in++;
     write_finish(_endpoints[EP_BULK_IN]);
-
     if (_abort_transfer_test == false) {
         // Receive more data from the host using the OUT endpoint.
         read_start(_endpoints[EP_BULK_OUT], _endpoint_buffs[EP_BULK_OUT], (*_endpoint_configs)[EP_BULK_OUT].max_packet);
@@ -757,6 +806,10 @@ void USBEndpointTester::_cb_bulk_in()
         write_start(_endpoints[EP_BULK_IN], _endpoint_buffs[EP_BULK_IN], (*_endpoint_configs)[EP_BULK_IN].max_packet);
         if (_num_packets_bulk_in_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(_endpoints[EP_BULK_IN]);
+            // Verify that buffer given in write_start is not used after the
+            // call to endpoint_abort(), by changing the buffer contents.
+            // The test will fail if the host receives new buffer content.
+            memset(_endpoint_buffs[EP_BULK_IN], FORBIDDEN_PAYLOAD_VALUE, (*_endpoint_configs)[EP_BULK_IN].max_packet);
         }
     }
 }
@@ -773,6 +826,10 @@ void USBEndpointTester::_cb_int_out()
     } else {
         // Abort the transfer if enough data was received.
         _num_packets_int_out_abort++;
+        if (_num_packets_int_out_abort == NUM_PACKETS_UNTIL_ABORT) {
+            // Set every byte of the buffer to a known value.
+            memset(_endpoint_buffs[EP_INT_OUT], EP_ABORT_BUFF_VALUE, (*_endpoint_configs)[EP_INT_OUT].max_packet);
+        }
         read_start(_endpoints[EP_INT_OUT], _endpoint_buffs[EP_INT_OUT], (*_endpoint_configs)[EP_INT_OUT].max_packet);
         if (_num_packets_int_out_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(_endpoints[EP_INT_OUT]);
@@ -797,6 +854,10 @@ void USBEndpointTester::_cb_int_in()
         write_start(_endpoints[EP_INT_IN], _endpoint_buffs[EP_INT_IN], (*_endpoint_configs)[EP_INT_IN].max_packet);
         if (_num_packets_int_in_abort == NUM_PACKETS_UNTIL_ABORT) {
             endpoint_abort(_endpoints[EP_INT_IN]);
+            // Verify that buffer given in write_start is not used after the
+            // call to endpoint_abort(), by changing the buffer contents.
+            // The test will fail if the host receives new buffer content.
+            memset(_endpoint_buffs[EP_INT_IN], FORBIDDEN_PAYLOAD_VALUE, (*_endpoint_configs)[EP_INT_IN].max_packet);
         }
     }
 }
diff --git a/TESTS/usb_device/basic/USBEndpointTester.h b/TESTS/usb_device/basic/USBEndpointTester.h
@@ -104,6 +104,7 @@ class USBEndpointTester: public USBDevice {
 private:
     const char *get_desc_string(const uint8_t *desc);
     bool _request_read_start(const setup_packet_t *setup);
+    bool _request_abort_buff_check(const setup_packet_t *setup);
 };
 
 #endif
diff --git a/TESTS/usb_device/basic/main.cpp b/TESTS/usb_device/basic/main.cpp
@@ -355,6 +355,7 @@ void ep_test_parallel_transfers_ctrl()
  * Given a USB device with multiple OUT/IN endpoint pairs
  * When a device aborts an in progress data transfer
  * Then no more data is transmitted
+ *     and endpoint buffer is correctly released on the device end
  */
 void ep_test_abort()
 {

Original file line number	Diff line number	Diff line change
`@@ -355,6 +355,7 @@ void ep_test_parallel_transfers_ctrl()`
`355`	`355`	`* Given a USB device with multiple OUT/IN endpoint pairs`
`356`	`356`	`* When a device aborts an in progress data transfer`
`357`	`357`	`* Then no more data is transmitted`
	`358`	`+ * and endpoint buffer is correctly released on the device end`
`358`	`359`	`*/`
`359`	`360`	`void ep_test_abort()`
`360`	`361`	`{`