usb: raw-gadget: fixes for 5.7-rcN

I've put all raw-gadget fixes in a series, please ignore the previous patches.

I've dropped the patches that change the ABI for now (those need more testing anyway).

Changes in v3:
- Dropped ABI breaking changes for .
- A few more comment fixes for uapi headers.
- Updated documentation.

Andrey Konovalov (5):
usb: raw-gadget: fix return value of ep read ioctls
usb: raw-gadget: improve uapi headers comments
usb: raw-gadget: fix gadget endpoint selection
usb: raw-gadget: support stalling/halting/wedging endpoints
usb: raw-gadget: documentation updates

 Documentation/usb/raw-gadget.rst       |  37 ++-
 drivers/usb/gadget/legacy/raw_gadget.c | 314 ++++++++++++++++++++-----
 include/uapi/linux/usb/raw_gadget.h    | 108 ++++++++-
 3 files changed, 377 insertions(+), 82 deletions(-)

2.26.2.526.g744177e7f7-goog

They must return the number of bytes transferred during the data stage.

---
 drivers/usb/gadget/legacy/raw_gadget.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index ca7d95bf7397..7b241992ad5a 100644
@@ -669,12 +669,14 @@ static int raw_ioctl_ep0_read(struct raw_dev *dev, unsigned long value)
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 	ret = raw_process_ep0_io(dev, &io, data, false);
-	if (ret)
+	if (ret < 0)
 		goto free;
 
 	length = min(io.length, (unsigned int)ret);
 	if (copy_to_user((void __user *)(value + sizeof(io)), data, length))
 		ret = -EFAULT;
+	else
+		ret = length;
 free:
 	kfree(data);
 	return ret;

@@ -964,12 +966,14 @@ static int raw_ioctl_ep_read(struct raw_dev *dev, unsigned long value)
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 	ret = raw_process_ep_io(dev, &io, data, false);
-	if (ret)
+	if (ret < 0)
 		goto free;
 
 	length = min(io.length, (unsigned int)ret);
 	if (copy_to_user((void __user *)(value + sizeof(io)), data, length))
 		ret = -EFAULT;
+	else
+		ret = length;
 free:
 	kfree(data);
 	return ret;

2.26.2.526.g744177e7f7-goog

Fix typo "trasferred" => "transferred".

Don't call USB requests URBs.

Fix comment style.

---
 include/uapi/linux/usb/raw_gadget.h | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

index ea375082b3ac..02885e021ee5 100644
@@ -115,11 +115,11 @@ struct usb_raw_ep_io {
 #define USB_RAW_IOCTL_EVENT_FETCH	_IOR('U', 2, struct usb_raw_event)
 
 /*
- * Queues an IN (OUT for READ) urb as a response to the last control request
- * received on endpoint 0, provided that was an IN (OUT for READ) request and
- * waits until the urb is completed. Copies received data to user for READ.
+ * Queues an IN (OUT for READ) request as a response to the last setup request
+ * received on endpoint 0 (provided that was an IN (OUT for READ) request), and
+ * waits until the request is completed. Copies received data to user for READ.
  * Accepts a pointer to the usb_raw_ep_io struct as an argument.
- * Returns length of trasferred data on success or negative error code on
+ * Returns length of transferred data on success or negative error code on
  * failure.
  */
 #define USB_RAW_IOCTL_EP0_WRITE		_IOW('U', 3, struct usb_raw_ep_io)

@@ -133,19 +133,20 @@ struct usb_raw_ep_io {
  */
 #define USB_RAW_IOCTL_EP_ENABLE		_IOW('U', 5, struct usb_endpoint_descriptor)
 
-/* Disables specified endpoint.
+/*
+ * Disables specified endpoint.
  * Accepts endpoint handle as an argument.
  * Returns 0 on success or negative error code on failure.
  */
 #define USB_RAW_IOCTL_EP_DISABLE	_IOW('U', 6, __u32)
 
 /*
- * Queues an IN (OUT for READ) urb as a response to the last control request
- * received on endpoint usb_raw_ep_io.ep, provided that was an IN (OUT for READ)
- * request and waits until the urb is completed. Copies received data to user
- * for READ.
+ * Queues an IN (OUT for READ) request as a response to the last setup request
+ * received on endpoint usb_raw_ep_io.ep (provided that was an IN (OUT for READ)
+ * request), and waits until the request is completed. Copies received data to
+ * user for READ.
  * Accepts a pointer to the usb_raw_ep_io struct as an argument.
- * Returns length of trasferred data on success or negative error code on
+ * Returns length of transferred data on success or negative error code on
  * failure.
  */
 #define USB_RAW_IOCTL_EP_WRITE		_IOW('U', 7, struct usb_raw_ep_io)

2.26.2.526.g744177e7f7-goog

Currently automatic gadget endpoint selection based on required features doesn't work. Raw Gadget tries iterating over the list of available endpoints and finding one that has the right direction and transfer type. Unfortunately selecting arbitrary gadget endpoints (even if they satisfy feature requirements) doesn't work, as (depending on the UDC driver) they might have fixed addresses, and one also needs to provide matching endpoint addresses in the descriptors sent to the host.

The composite framework deals with this by assigning endpoint addresses in usb_ep_autoconfig() before enumeration starts. This approach won't work with Raw Gadget as the endpoints are supposed to be enabled after a set_configuration/set_interface request from the host, so it's too late to patch the endpoint descriptors that had already been sent to the host.

For Raw Gadget we take another approach. Similarly to GadgetFS, we allow the user to make the decision as to which gadget endpoints to use.

This patch adds another Raw Gadget ioctl USB_RAW_IOCTL_EPS_INFO that exposes information about all non-control endpoints that a currently connected UDC has. This information includes endpoints addresses, as well as their capabilities and limits to allow the user to choose the most fitting gadget endpoint.

The USB_RAW_IOCTL_EP_ENABLE ioctl is updated to use the proper endpoint validation routine usb_gadget_ep_match_desc().

These changes affect the portability of the gadgets that use Raw Gadget when running on different UDCs. Nevertheless, as long as the user relies on the information provided by USB_RAW_IOCTL_EPS_INFO to dynamically choose endpoint addresses, UDC-agnostic gadgets can still be written with Raw Gadget.

---
 Documentation/usb/raw-gadget.rst       |   5 +-
 drivers/usb/gadget/legacy/raw_gadget.c | 187 ++++++++++++++++---------
 include/uapi/linux/usb/raw_gadget.h    |  72 +++++++++-
 3 files changed, 194 insertions(+), 70 deletions(-)

index 9e78cb858f86..4af8b1f15574 100644
@@ -27,9 +27,8 @@ differences are:
 3. Raw Gadget provides a way to select a UDC device/driver to bind to,
    while GadgetFS currently binds to the first available UDC.
 
-4. Raw Gadget uses predictable endpoint names (handles) across different
-   UDCs (as long as UDCs have enough endpoints of each required transfer
-   type).
+4. Raw Gadget explicitly exposes information about endpoints addresses and
+   capabilities allowing a user to write UDC-agnostic gadgets.
 
 5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.
 

2.26.2.526.g744177e7f7-goog

you're adding a new IOCTL, how is this a fix?

balbi

It's a fix in a sense that without this patch a user can't make raw-gadget work with other UDCs besides dummy, and with this patch it works (at least with dwc2 I'm currently using for tests).

I guess I could split this patch into two, one that removes the failed attempt to auto-assign endpoints, and another one that adds the new ioctl. Should I do that?

Raw Gadget is currently unable to stall/halt/wedge gadget endpoints, which is required for proper emulation of certain USB classes.

This patch adds a few more ioctls:

- USB_RAW_IOCTL_EP0_STALL allows to stall control endpoint #0 when there's a pending setup request for it.
- USB_RAW_IOCTL_SET/CLEAR_HALT/WEDGE allow to set/clear halt/wedge status on non-control non-isochronous endpoints.

---
 Documentation/usb/raw-gadget.rst       |   2 -
 drivers/usb/gadget/legacy/raw_gadget.c | 131 ++++++++++++++++++++++++-
 include/uapi/linux/usb/raw_gadget.h    |  15 +++
 3 files changed, 144 insertions(+), 4 deletions(-)

index 4af8b1f15574..3b3d78e850b2 100644
@@ -52,8 +52,6 @@ The typical usage of Raw Gadget looks like:
 Potential future improvements
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- Implement ioctl's for setting/clearing halt status on endpoints.
-
 - Reporting more events (suspend, resume, etc.) through
   USB_RAW_IOCTL_EVENT_FETCH.
 

index c89f6341229c..0be685272eb1 100644
@@ -231,4 +231,19 @@ struct usb_raw_eps_info {
  */
 #define USB_RAW_IOCTL_EPS_INFO		_IOR('U', 11, struct usb_raw_eps_info)
 
+/*
+ * Stalls a pending control request on endpoint 0.
+ * Returns 0 on success or negative error code on failure.
+ */
+#define USB_RAW_IOCTL_EP0_STALL		_IO('U', 12)
+
+/*
+ * Sets or clears halt or wedge status of the endpoint.
+ * Accepts endpoint handle as an argument.
+ * Returns 0 on success or negative error code on failure.
+ */
+#define USB_RAW_IOCTL_EP_SET_HALT	_IOW('U', 13, __u32)
+#define USB_RAW_IOCTL_EP_CLEAR_HALT	_IOW('U', 14, __u32)
+#define USB_RAW_IOCTL_EP_SET_WEDGE	_IOW('U', 15, __u32)
+
 #endif /* _UAPI__LINUX_USB_RAW_GADGET_H */

2.26.2.526.g744177e7f7-goog

Mention the issue with fixed UDC addresses.

Links external examples and test suite.

Add more implmenetation details and potential improvements.

---
 Documentation/usb/raw-gadget.rst | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

index 3b3d78e850b2..68d879a8009e 100644
@@ -49,10 +49,36 @@ The typical usage of Raw Gadget looks like:
    Raw Gadget and react to those depending on what kind of USB device
    needs to be emulated.
 
+Note, that some UDC drivers have fixed addresses assigned to endpoints, and
+therefore arbitrary endpoint addresses can't be used in the descriptors.
+Nevertheles, Raw Gadget provides a UDC-agnostic way to write USB gadgets.
+Once a USB_RAW_EVENT_CONNECT event is received via USB_RAW_IOCTL_EVENT_FETCH,
+the USB_RAW_IOCTL_EPS_INFO ioctl can be used to find out information about
+endpoints that the UDC driver has. Based on that information, the user must
+chose UDC endpoints that will be used for the gadget being emulated, and
+properly assign addresses in endpoint descriptors.
+
+You can find usage examples (along with a test suite) here:
+
+https://github.com/xairy/raw-gadget
+
+Internal details
+~~~~~~~~~~~~~~~~
+
+Currently every endpoint read/write ioctl submits a USB request and waits until
+its completion. This is the desired mode for coverage-guided fuzzing (as we'd
+like all USB request processing happen during the lifetime of a syscall),
+and must be kept in the implementation. (This might be slow for real world
+applications, thus the O_NONBLOCK improvement suggestion below.)
+
 Potential future improvements
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- Reporting more events (suspend, resume, etc.) through
-  USB_RAW_IOCTL_EVENT_FETCH.
+- Report more events (suspend, resume, etc.) through USB_RAW_IOCTL_EVENT_FETCH.
 
 - Support O_NONBLOCK I/O.
+
+- Support USB 3 features (accept SS endpoint companion descriptor when
+  enabling endpoints; allow providing stream_id for bulk transfers).
+
+- Support ISO transfer features (expose frame_number for completed requests).

2.26.2.526.g744177e7f7-goog