Skip to contentSkip to conversation

usb: gadget: add raw-gadget interface

PATCH Superseded Andrey Konovalov 2 participants 7 messages Dec 11, 2019 – Dec 18, 2019

Subsystems (3)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v3 cover usb: gadget: add raw-gadget interface

This patchset (currently a single patch) adds a new userspace interface for the USB Gadget subsystem called USB Raw Gadget (I don't mind changing the name to something else if there are better ideas). This is what currently being used to enable coverage-buided USB fuzzing with syzkaller:

https://github.com/google/syzkaller/blob/master/docs/linux/external_fuzzing_usb.md

Initially I was using GadgetFS (together with the Dummy HCD/UDC module) to perform emulation of USB devices for fuzzing, but later switched to a custom written interface. The incentive to implement a different interface was to provide a somewhat raw and direct access to the USB Gadget layer for the userspace, where every USB request is passed to the userspace to get a response. See documentation for the list of differences between Raw Gadget and GadgetFS.

This patchset has been pushed to the public Linux kernel Gerrit instance:

https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2144

Changes v2 -> v3:
- Updated device path in documentation.
- Changed usb_raw_init struct layout to make it the same for 32 bit compat mode.
- Added compat_ioctl to raw_fops.
- Changed raw_ioctl_init() to return EINVAL for invalid USB speeds, except for USB_SPEED_UNKNOWN, which defaults to USB_SPEED_HIGH.
- Reject endpoints with maxpacket = 0 in raw_ioctl_ep_enable().

Changes v1 -> v2:
- Moved raw.c to legacy/.
- Changed uapi header to use __u* types.
- Switched from debugfs entry to a misc device.
- Changed raw_dev from refcount to kref.
- Moved UDC_NAME_LENGTH_MAX to uapi headers.
- Used usb_endpoint_type() and usb_endpoint_dir_in/out() functions instead of open coding them.
- Added "WITH Linux-syscall-note" to SPDX id in the uapi header.
- Removed pr_err() if case dev_new() fails.
- Reduced the number of debugging messages.

Andrey Konovalov (1):
usb: gadget: add raw-gadget interface

 Documentation/usb/index.rst         |    1 +
 Documentation/usb/raw-gadget.rst    |   59 ++
 drivers/usb/gadget/legacy/Kconfig   |    8 +
 drivers/usb/gadget/legacy/Makefile  |    1 +
 drivers/usb/gadget/legacy/raw.c     | 1070 +++++++++++++++++++++++++++
 include/uapi/linux/usb/raw_gadget.h |  167 +++++
 6 files changed, 1306 insertions(+)
 create mode 100644 Documentation/usb/raw-gadget.rst
 create mode 100644 drivers/usb/gadget/legacy/raw.c
 create mode 100644 include/uapi/linux/usb/raw_gadget.h
Signature · 1 line
2.24.1.735.g03f4e72817-goog
#1 lore raw

PATCH v3 1/1 usb: gadget: add raw-gadget interface

USB Raw Gadget is a kernel module that provides a userspace interface for the USB Gadget subsystem. Essentially it allows to emulate USB devices from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is currently a strictly debugging feature and shouldn't be used in production.

Raw Gadget is similar to GadgetFS, but provides a more low-level and direct access to the USB Gadget layer for the userspace. The key differences are:

1. Every USB request is passed to the userspace to get a response, while GadgetFS responds to some USB requests internally based on the provided descriptors. However note, that the UDC driver might respond to some requests on its own and never forward them to the Gadget layer.

2. GadgetFS performs some sanity checks on the provided USB descriptors, while Raw Gadget allows you to provide arbitrary data as responses to USB requests.

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).

5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.

---
 Documentation/usb/index.rst         |    1 +
 Documentation/usb/raw-gadget.rst    |   59 ++
 drivers/usb/gadget/legacy/Kconfig   |    8 +
 drivers/usb/gadget/legacy/Makefile  |    1 +
 drivers/usb/gadget/legacy/raw.c     | 1070 +++++++++++++++++++++++++++
 include/uapi/linux/usb/raw_gadget.h |  167 +++++
 6 files changed, 1306 insertions(+)
 create mode 100644 Documentation/usb/raw-gadget.rst
 create mode 100644 drivers/usb/gadget/legacy/raw.c
 create mode 100644 include/uapi/linux/usb/raw_gadget.h
Documentation/usb/index.rst +1 -0
index e55386a4abfb..90310e2a0c1f 100644
@@ -22,6 +22,7 @@ USB support
     misc_usbsevseg
     mtouchusb
     ohci
+    raw-gadget
     rio
     usbip_protocol
     usbmon
Documentation/usb/raw-gadget.rst new file +59 -0

59 lines not on this page · raw message

drivers/usb/gadget/legacy/Kconfig +8 -0
index 119a4e47681f..8c947edf7515 100644
@@ -489,3 +489,11 @@ config USB_G_WEBCAM
 
 	  Say "y" to link the driver statically, or "m" to build a
 	  dynamically linked module called "g_webcam".
+
+config USB_RAW_GADGET
+	tristate "USB Raw Gadget"
+	help
+	  USB Raw Gadget is a kernel module that provides a userspace interface
+	  for the USB Gadget subsystem. Essentially it allows to emulate USB
+	  devices from userspace. See Documentation/usb/raw-gadget.rst for
+	  details.
drivers/usb/gadget/legacy/Makefile +1 -0
index abd0c3e66a05..799feefeee58 100644
@@ -43,3 +43,4 @@ obj-$(CONFIG_USB_G_WEBCAM)	+= g_webcam.o
 obj-$(CONFIG_USB_G_NCM)		+= g_ncm.o
 obj-$(CONFIG_USB_G_ACM_MS)	+= g_acm_ms.o
 obj-$(CONFIG_USB_GADGET_TARGET)	+= tcm_usb_gadget.o
+obj-$(CONFIG_USB_RAW_GADGET)	+= raw.o
drivers/usb/gadget/legacy/raw.c new file +1070 -0

1,070 lines not on this page · raw message

include/uapi/linux/usb/raw_gadget.h new file +167 -0

167 lines not on this page · raw message

Signature · 1 line
2.24.1.735.g03f4e72817-goog
#2 lore raw
Quoted 19 lines · Andrey Konovalov

USB Raw Gadget is a kernel module that provides a userspace interface for the USB Gadget subsystem. Essentially it allows to emulate USB devices from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is currently a strictly debugging feature and shouldn't be used in production.

Raw Gadget is similar to GadgetFS, but provides a more low-level and direct access to the USB Gadget layer for the userspace. The key differences are:

1. Every USB request is passed to the userspace to get a response, while GadgetFS responds to some USB requests internally based on the provided descriptors. However note, that the UDC driver might respond to some requests on its own and never forward them to the Gadget layer.

2. GadgetFS performs some sanity checks on the provided USB descriptors, while Raw Gadget allows you to provide arbitrary data as responses to USB requests.

Quoted 8 lines

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).

5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.

Looks good to me, only minor comments below.

Quoted 96 lines
---
 Documentation/usb/index.rst         |    1 +
 Documentation/usb/raw-gadget.rst    |   59 ++
 drivers/usb/gadget/legacy/Kconfig   |    8 +
 drivers/usb/gadget/legacy/Makefile  |    1 +
 drivers/usb/gadget/legacy/raw.c     | 1070 +++++++++++++++++++++++++++
 include/uapi/linux/usb/raw_gadget.h |  167 +++++
 6 files changed, 1306 insertions(+)
 create mode 100644 Documentation/usb/raw-gadget.rst
 create mode 100644 drivers/usb/gadget/legacy/raw.c
 create mode 100644 include/uapi/linux/usb/raw_gadget.h
Documentation/usb/index.rst +1 -0
index e55386a4abfb..90310e2a0c1f 100644
@@ -22,6 +22,7 @@ USB support
     misc_usbsevseg
     mtouchusb
     ohci
+    raw-gadget
     rio
     usbip_protocol
     usbmon
Documentation/usb/raw-gadget.rst new file +59 -0
new file mode 100644
index 000000000000..cbedf5451ed3
@@ -0,0 +1,59 @@
+==============
+USB Raw Gadget
+==============
+
+USB Raw Gadget is a kernel module that provides a userspace interface for
+the USB Gadget subsystem. Essentially it allows to emulate USB devices
+from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is
+currently a strictly debugging feature and shouldn't be used in
+production, use GadgetFS instead.
+
+Comparison to GadgetFS
+~~~~~~~~~~~~~~~~~~~~~~
+
+Raw Gadget is similar to GadgetFS, but provides a more low-level and
+direct access to the USB Gadget layer for the userspace. The key
+differences are:
+
+1. Every USB request is passed to the userspace to get a response, while
+   GadgetFS responds to some USB requests internally based on the provided
+   descriptors. However note, that the UDC driver might respond to some
+   requests on its own and never forward them to the Gadget layer.
+
+2. GadgetFS performs some sanity checks on the provided USB descriptors,
+   while Raw Gadget allows you to provide arbitrary data as responses to
+   USB requests.
+
+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).
+
+5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.
+
+Userspace interface
+~~~~~~~~~~~~~~~~~~~
+
+To create a Raw Gadget instance open /dev/raw-gadget. Multiple raw-gadget
+instances (bound to different UDCs) can be used at the same time. The
+interaction with the opened file happens through the ioctl() calls, see
+comments in include/uapi/linux/usb/raw_gadget.h for details.
+
+The typical usage of Raw Gadget looks like:
+
+1. Open Raw Gadget instance via /dev/raw-gadget.
+2. Initialize the instance via USB_RAW_IOCTL_INIT.
+3. Launch the instance with USB_RAW_IOCTL_RUN.
+4. In a loop issue USB_RAW_IOCTL_EVENT_FETCH calls to receive events from
+   Raw Gadget and react to those depending on what kind of USB device
+   needs to be emulated.
+
+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.
drivers/usb/gadget/legacy/Kconfig +0 -0
index 119a4e47681f..8c947edf7515 100644
@@ -489,3 +489,11 @@ config USB_G_WEBCAM
 

drivers/usb/gadget/legacy/Kconfig config USB_G_WEBCAM · line 499

Quoted 10 lines
drivers/usb/gadget/legacy/Kconfig +8 -0
 	  Say "y" to link the driver statically, or "m" to build a
 	  dynamically linked module called "g_webcam".
+
+config USB_RAW_GADGET
+	tristate "USB Raw Gadget"
+	help
+	  USB Raw Gadget is a kernel module that provides a userspace interface
+	  for the USB Gadget subsystem. Essentially it allows to emulate USB
+	  devices from userspace. See Documentation/usb/raw-gadget.rst for
+	  details.

No help text saying what the module name would be if selected as one?

drivers/usb/gadget/legacy/Makefile obj-$(CONFIG_USB_G_WEBCAM) += g_webcam.o · line 46

Quoted 9 lines
drivers/usb/gadget/legacy/Makefile +1 -0
index abd0c3e66a05..799feefeee58 100644
@@ -43,3 +43,4 @@ obj-$(CONFIG_USB_G_WEBCAM)	+= g_webcam.o
 obj-$(CONFIG_USB_G_NCM)		+= g_ncm.o
 obj-$(CONFIG_USB_G_ACM_MS)	+= g_acm_ms.o
 obj-$(CONFIG_USB_GADGET_TARGET)	+= tcm_usb_gadget.o
+obj-$(CONFIG_USB_RAW_GADGET)	+= raw.o

raw.ko? I think we already have a raw.ko in the kernel, shouldn't this be usb_raw_gadget.ko or something?

Yeah, we do, look at drivers/char/raw.c. This needs to be changed, I can't take it as-is otherwise the builders will croak on it.

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +0 -0
new file mode 100644
index 000000000000..fcfa2ebc103e

drivers/usb/gadget/legacy/raw.c line 9

Quoted 9 lines
drivers/usb/gadget/legacy/raw.c new file +9 -0
@@ -0,0 +1,1070 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Raw Gadget driver.
+ * See Documentation/usb/raw-gadget.rst for more details.
+ *
+ * Andrey Konovalov <[email protected]>
+ */
+
+#define pr_fmt(fmt) "raw: %s: " fmt, __func__

"raw gadget"?

This a driver, why do you ever need to do a pr_* call and not a dev_* instead?

Quoted 12 lines
drivers/usb/gadget/legacy/raw.c new file +12 -0
+
+#include <linux/compiler.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/kref.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/semaphore.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/wait.h>

drivers/usb/gadget/legacy/raw.c line 31

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+
+#include <linux/usb.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/ch11.h>
+#include <linux/usb/gadget.h>
+
+#include <uapi/linux/usb/raw_gadget.h>
+
+#define	DRIVER_DESC "USB Raw Gadget"
+#define DRIVER_NAME "raw-gadget"

You only use this in one place. You can use it in the definition of the misc driver name also, right?

drivers/usb/gadget/legacy/raw.c line 43

Quoted 12 lines
drivers/usb/gadget/legacy/raw.c new file +12 -0
+
+MODULE_DESCRIPTION(DRIVER_DESC);
+MODULE_AUTHOR("Andrey Konovalov");
+MODULE_LICENSE("GPL");
+
+/*----------------------------------------------------------------------*/
+
+#define RAW_EVENT_QUEUE_SIZE	128
+
+struct raw_event_queue {
+	spinlock_t		lock;
+	struct semaphore	sema;

Two locks for the same structure? That feels "wrong". Why is this? document the heck out of it if it is required please.

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	struct usb_raw_event	*events[RAW_EVENT_QUEUE_SIZE];
+	int			size;
+};
+
+static void raw_event_queue_init(struct raw_event_queue *queue)
+{
+	spin_lock_init(&queue->lock);
+	sema_init(&queue->sema, 0);
+	queue->size = 0;
+}

drivers/usb/gadget/legacy/raw.c line 63

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+
+static int raw_event_queue_add(struct raw_event_queue *queue,
+	enum usb_raw_event_type type, size_t length, const void *data)
+{
+	unsigned long flags;
+	struct usb_raw_event *event;
+
+	spin_lock_irqsave(&queue->lock, flags);
+	if (queue->size >= RAW_EVENT_QUEUE_SIZE) {
+		pr_err("too many events\n");

What can someone do with this error?

dev_err() to show exactly where it came from?

Quoted 18 lines
drivers/usb/gadget/legacy/raw.c new file +18 -0
+		spin_unlock_irqrestore(&queue->lock, flags);
+		return -ENOMEM;
+	}
+	event = kmalloc(sizeof(*event) + length, GFP_ATOMIC);
+	if (!event) {
+		spin_unlock_irqrestore(&queue->lock, flags);
+		return -ENOMEM;
+	}
+	event->type = type;
+	event->length = length;
+	if (event->length)
+		memcpy(&event->data[0], data, length);
+	queue->events[queue->size] = event;
+	queue->size++;
+	up(&queue->sema);
+	spin_unlock_irqrestore(&queue->lock, flags);
+	return 0;
+}

drivers/usb/gadget/legacy/raw.c line 91

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+
+static struct usb_raw_event *raw_event_queue_fetch(
+				struct raw_event_queue *queue)
+{
+	unsigned long flags;
+	struct usb_raw_event *event;
+
+	if (down_interruptible(&queue->sema))
+		return NULL;
+	spin_lock_irqsave(&queue->lock, flags);

You just locked twice?

drivers/usb/gadget/legacy/raw.c line 93

Quoted 2 lines
drivers/usb/gadget/legacy/raw.c new file +2 -0
+	if (WARN_ON(!queue->size))
+		return NULL;

It's as if you want to trigger syzbot :)

Just log it and return an error.

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+	event = queue->events[0];
+	queue->size--;
+	memmove(&queue->events[0], &queue->events[1],
+			queue->size * sizeof(queue->events[0]));
+	spin_unlock_irqrestore(&queue->lock, flags);

drivers/usb/gadget/legacy/raw.c line 108

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	return event;
+}
+
+static void raw_event_queue_destroy(struct raw_event_queue *queue)
+{
+	int i;
+
+	for (i = 0; i < queue->size; i++)
+		kfree(queue->events[i]);
+	queue->size = 0;

No locking needed?

Quoted 28 lines
drivers/usb/gadget/legacy/raw.c new file +28 -0
+}
+
+/*----------------------------------------------------------------------*/
+
+struct raw_dev;
+
+#define USB_RAW_MAX_ENDPOINTS 32
+
+enum ep_state {
+	STATE_EP_DISABLED,
+	STATE_EP_ENABLED,
+};
+
+struct raw_ep {
+	struct raw_dev		*dev;
+	enum ep_state		state;
+	struct usb_ep		*ep;
+	struct usb_request	*req;
+	bool			urb_queued;
+	bool			disabling;
+	ssize_t			status;
+};
+
+enum dev_state {
+	STATE_DEV_INVALID = 0,
+	STATE_DEV_OPENED,
+	STATE_DEV_INITIALIZED,
+	STATE_DEV_RUNNING,

drivers/usb/gadget/legacy/raw.c line 146

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	STATE_DEV_CLOSED,
+	STATE_DEV_FAILED
+};
+
+struct raw_dev {
+	struct kref			count;
+	spinlock_t			lock;
+
+	const char			*udc_name;
+	struct usb_gadget_driver	driver;

A dev embeds a driver?

Not a pointer?

But you have a kref, so the reference count of this object is there, right?

Quoted 4 lines
drivers/usb/gadget/legacy/raw.c new file +4 -0
+
+	/* Protected by lock: */
+	enum dev_state			state;
+	bool				gadget_registered;

drivers/usb/gadget/legacy/raw.c line 160

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	struct usb_gadget		*gadget;
+	struct usb_request		*req;
+	bool				ep0_in_pending;
+	bool				ep0_out_pending;
+	bool				ep0_urb_queued;
+	ssize_t				ep0_status;
+	struct raw_ep			eps[USB_RAW_MAX_ENDPOINTS];
+
+	struct completion		ep0_done;
+	struct raw_event_queue		queue;

No pointer to your misc device? I think you need that for logging.

Quoted 113 lines
drivers/usb/gadget/legacy/raw.c new file +113 -0
+};
+
+static struct raw_dev *dev_new(void)
+{
+	struct raw_dev *dev;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+	/* Matches kref_put() in raw_release(). */
+	kref_init(&dev->count);
+	spin_lock_init(&dev->lock);
+	init_completion(&dev->ep0_done);
+	raw_event_queue_init(&dev->queue);
+	return dev;
+}
+
+static void dev_free(struct kref *kref)
+{
+	struct raw_dev *dev = container_of(kref, struct raw_dev, count);
+	int i;
+
+	kfree(dev->udc_name);
+	kfree(dev->driver.udc_name);
+	if (dev->req) {
+		if (dev->ep0_urb_queued)
+			usb_ep_dequeue(dev->gadget->ep0, dev->req);
+		usb_ep_free_request(dev->gadget->ep0, dev->req);
+	}
+	raw_event_queue_destroy(&dev->queue);
+	for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
+		if (dev->eps[i].state != STATE_EP_ENABLED)
+			continue;
+		usb_ep_disable(dev->eps[i].ep);
+		usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
+		kfree(dev->eps[i].ep->desc);
+		dev->eps[i].state = STATE_EP_DISABLED;
+	}
+	kfree(dev);
+}
+
+/*----------------------------------------------------------------------*/
+
+static int raw_queue_event(struct raw_dev *dev,
+	enum usb_raw_event_type type, size_t length, const void *data)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	ret = raw_event_queue_add(&dev->queue, type, length, data);
+	if (ret < 0) {
+		spin_lock_irqsave(&dev->lock, flags);
+		dev->state = STATE_DEV_FAILED;
+		spin_unlock_irqrestore(&dev->lock, flags);
+	}
+	return ret;
+}
+
+static void gadget_ep0_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct raw_dev *dev = req->context;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (req->status)
+		dev->ep0_status = req->status;
+	else
+		dev->ep0_status = req->actual;
+	if (dev->ep0_in_pending)
+		dev->ep0_in_pending = false;
+	else
+		dev->ep0_out_pending = false;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	complete(&dev->ep0_done);
+}
+
+static int gadget_bind(struct usb_gadget *gadget,
+			struct usb_gadget_driver *driver)
+{
+	int ret = 0;
+	struct raw_dev *dev = container_of(driver, struct raw_dev, driver);
+	struct usb_request *req;
+	unsigned long flags;
+
+	if (strcmp(gadget->name, dev->udc_name) != 0)
+		return -ENODEV;
+
+	set_gadget_data(gadget, dev);
+	req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
+	if (!req) {
+		pr_err("usb_ep_alloc_request failed\n");
+		set_gadget_data(gadget, NULL);
+		return -ENOMEM;
+	}
+
+	spin_lock_irqsave(&dev->lock, flags);
+	dev->req = req;
+	dev->req->context = dev;
+	dev->req->complete = gadget_ep0_complete;
+	dev->gadget = gadget;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	/* Matches kref_put() in gadget_unbind(). */
+	kref_get(&dev->count);
+
+	ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL);
+	if (ret < 0)
+		pr_err("failed to queue event\n");
+
+	return ret;
+}
+

drivers/usb/gadget/legacy/raw.c line 283

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+static void gadget_unbind(struct usb_gadget *gadget)
+{
+	struct raw_dev *dev = get_gadget_data(gadget);
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	set_gadget_data(gadget, NULL);
+	spin_unlock_irqrestore(&dev->lock, flags);
+	/* Matches kref_get() in gadget_bind(). */
+	kref_put(&dev->count, dev_free);

What protects the kref from being called 'put' twice on the same pointer at the same time? There should be some lock somewhere, right?

Quoted 42 lines
drivers/usb/gadget/legacy/raw.c new file +42 -0
+}
+
+static int gadget_setup(struct usb_gadget *gadget,
+			const struct usb_ctrlrequest *ctrl)
+{
+	int ret = 0;
+	struct raw_dev *dev = get_gadget_data(gadget);
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_err("ignoring, device is not running\n");
+		ret = -ENODEV;
+		goto out_unlock;
+	}
+	if (dev->ep0_in_pending || dev->ep0_out_pending) {
+		pr_debug("stalling, already have pending request\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength)
+		dev->ep0_in_pending = true;
+	else
+		dev->ep0_out_pending = true;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	ret = raw_queue_event(dev, USB_RAW_EVENT_CONTROL, sizeof(*ctrl), ctrl);
+	if (ret < 0)
+		pr_err("failed to queue event\n");
+	goto out;
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+out:
+	return ret;
+}
+
+/* These are currently ununsed but present in case UDC driver requires them. */
+static void gadget_disconnect(struct usb_gadget *gadget) { }
+static void gadget_suspend(struct usb_gadget *gadget) { }
+static void gadget_resume(struct usb_gadget *gadget) { }
+static void gadget_reset(struct usb_gadget *gadget) { }

drivers/usb/gadget/legacy/raw.c line 335

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+
+/*----------------------------------------------------------------------*/
+
+static int raw_open(struct inode *inode, struct file *fd)
+{
+	struct raw_dev *dev;
+
+	dev = dev_new();
+	if (!dev) {
+		pr_err("failed to created device");

You should have access to your misc device here, so dev_err()?

Quoted 18 lines
drivers/usb/gadget/legacy/raw.c new file +18 -0
+		return -ENOMEM;
+	}
+	fd->private_data = dev;
+	dev->state = STATE_DEV_OPENED;
+	return 0;
+}
+
+static int raw_release(struct inode *inode, struct file *fd)
+{
+	int ret = 0;
+	struct raw_dev *dev = fd->private_data;
+	unsigned long flags;
+	bool unregister = false;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	dev->state = STATE_DEV_CLOSED;
+	if (!dev->gadget) {
+		spin_unlock_irqrestore(&dev->lock, flags);

drivers/usb/gadget/legacy/raw.c line 363

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+		goto out_put;
+	}
+	if (dev->gadget_registered)
+		unregister = true;
+	dev->gadget_registered = false;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	if (unregister) {
+		ret = usb_gadget_unregister_driver(&dev->driver);
+		WARN_ON(ret != 0);

you want to crash? :)

Quoted 55 lines
drivers/usb/gadget/legacy/raw.c new file +55 -0
+		/* Matches kref_get() in raw_ioctl_run(). */
+		kref_put(&dev->count, dev_free);
+	}
+
+out_put:
+	/* Matches dev_new() in raw_open(). */
+	kref_put(&dev->count, dev_free);
+	return ret;
+}
+
+/*----------------------------------------------------------------------*/
+
+static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	struct usb_raw_init arg;
+	char *udc_driver_name;
+	char *udc_device_name;
+	unsigned long flags;
+
+	ret = copy_from_user(&arg, (void __user *)value, sizeof(arg));
+	if (ret)
+		return ret;
+
+	switch (arg.speed) {
+	case USB_SPEED_UNKNOWN:
+		arg.speed = USB_SPEED_HIGH;
+		break;
+	case USB_SPEED_LOW:
+	case USB_SPEED_FULL:
+	case USB_SPEED_HIGH:
+	case USB_SPEED_SUPER:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	udc_driver_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
+	if (!udc_driver_name)
+		return -ENOMEM;
+	ret = strscpy(udc_driver_name, &arg.driver_name[0],
+				UDC_NAME_LENGTH_MAX);
+	if (ret < 0) {
+		kfree(udc_driver_name);
+		return ret;
+	}
+	ret = 0;
+
+	udc_device_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
+	if (!udc_device_name) {
+		kfree(udc_driver_name);
+		return -ENOMEM;
+	}
+	ret = strscpy(udc_device_name, &arg.device_name[0],
+				UDC_NAME_LENGTH_MAX);

drivers/usb/gadget/legacy/raw.c line 428

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	if (ret < 0) {
+		kfree(udc_driver_name);
+		kfree(udc_device_name);
+		return ret;
+	}
+	ret = 0;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_OPENED) {
+		pr_debug("fail, device is not opened\n");

dev_dbg().

Quoted 28 lines
drivers/usb/gadget/legacy/raw.c new file +28 -0
+		kfree(udc_driver_name);
+		kfree(udc_device_name);
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	dev->udc_name = udc_driver_name;
+
+	dev->driver.function = DRIVER_DESC;
+	dev->driver.max_speed = arg.speed;
+	dev->driver.setup = gadget_setup;
+	dev->driver.disconnect = gadget_disconnect;
+	dev->driver.bind = gadget_bind;
+	dev->driver.unbind = gadget_unbind;
+	dev->driver.suspend = gadget_suspend;
+	dev->driver.resume = gadget_resume;
+	dev->driver.reset = gadget_reset;
+	dev->driver.driver.name = DRIVER_NAME;
+	dev->driver.udc_name = udc_device_name;
+	dev->driver.match_existing_only = 1;
+
+	dev->state = STATE_DEV_INITIALIZED;
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_run(struct raw_dev *dev, unsigned long value)

drivers/usb/gadget/legacy/raw.c line 466

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+{
+	int ret = 0;
+	unsigned long flags;
+
+	if (value)
+		return -EINVAL;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_INITIALIZED) {
+		pr_debug("fail, device is not initialized\n");

dev_dbg()

drivers/usb/gadget/legacy/raw.c line 476

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	ret = usb_gadget_probe_driver(&dev->driver);
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (ret) {
+		pr_err("fail, usb_gadget_probe_driver returned %d\n", ret);

dev_err()

same goes for everywhere, no need for any pr_* calls in this driver.

Quoted 562 lines
drivers/usb/gadget/legacy/raw.c new file +562 -0
+		dev->state = STATE_DEV_FAILED;
+		goto out_unlock;
+	}
+	dev->gadget_registered = true;
+	dev->state = STATE_DEV_RUNNING;
+	/* Matches kref_put() in raw_release(). */
+	kref_get(&dev->count);
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_event_fetch(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	struct usb_raw_event arg;
+	unsigned long flags;
+	struct usb_raw_event *event;
+	uint32_t length;
+
+	ret = copy_from_user(&arg, (void __user *)value, sizeof(arg));
+	if (ret)
+		return ret;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		spin_unlock_irqrestore(&dev->lock, flags);
+		return -EINVAL;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		spin_unlock_irqrestore(&dev->lock, flags);
+		return -EBUSY;
+	}
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	event = raw_event_queue_fetch(&dev->queue);
+	if (!event) {
+		pr_debug("event fetching interrupted\n");
+		return -EINTR;
+	}
+	length = min(arg.length, event->length);
+	ret = copy_to_user((void __user *)value, event,
+				sizeof(*event) + length);
+	return ret;
+}
+
+static void *raw_alloc_io_data(struct usb_raw_ep_io *io, void __user *ptr,
+				bool get_from_user)
+{
+	int ret;
+	void *data;
+
+	ret = copy_from_user(io, ptr, sizeof(*io));
+	if (ret)
+		return ERR_PTR(ret);
+	if (io->ep >= USB_RAW_MAX_ENDPOINTS)
+		return ERR_PTR(-EINVAL);
+	if (!usb_raw_io_flags_valid(io->flags))
+		return ERR_PTR(-EINVAL);
+	if (io->length > PAGE_SIZE)
+		return ERR_PTR(-EINVAL);
+	if (get_from_user)
+		data = memdup_user(ptr + sizeof(*io), io->length);
+	else {
+		data = kmalloc(io->length, GFP_KERNEL);
+		if (!data)
+			data = ERR_PTR(-ENOMEM);
+	}
+	return data;
+}
+
+static int raw_process_ep0_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
+				void *data, bool in)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (dev->ep0_urb_queued) {
+		pr_debug("fail, urb already queued\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if ((in && !dev->ep0_in_pending) ||
+			(!in && !dev->ep0_out_pending)) {
+		pr_debug("fail, wrong direction\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (WARN_ON(in && dev->ep0_out_pending)) {
+		ret = -ENODEV;
+		dev->state = STATE_DEV_FAILED;
+		goto out_done;
+	}
+	if (WARN_ON(!in && dev->ep0_in_pending)) {
+		ret = -ENODEV;
+		dev->state = STATE_DEV_FAILED;
+		goto out_done;
+	}
+
+	dev->req->buf = data;
+	dev->req->length = io->length;
+	dev->req->zero = usb_raw_io_flags_zero(io->flags);
+	dev->ep0_urb_queued = true;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	ret = usb_ep_queue(dev->gadget->ep0, dev->req, GFP_ATOMIC);
+	if (ret) {
+		pr_err("fail, usb_ep_queue returned %d\n", ret);
+		spin_lock_irqsave(&dev->lock, flags);
+		dev->state = STATE_DEV_FAILED;
+		goto out_done;
+	}
+
+	ret = wait_for_completion_interruptible(&dev->ep0_done);
+	if (ret) {
+		pr_debug("wait interrupted\n");
+		usb_ep_dequeue(dev->gadget->ep0, dev->req);
+		wait_for_completion(&dev->ep0_done);
+		spin_lock_irqsave(&dev->lock, flags);
+		goto out_done;
+	}
+
+	spin_lock_irqsave(&dev->lock, flags);
+	ret = dev->ep0_status;
+
+out_done:
+	dev->ep0_urb_queued = false;
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_ep0_write(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	void *data;
+	struct usb_raw_ep_io io;
+
+	data = raw_alloc_io_data(&io, (void __user *)value, true);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+	ret = raw_process_ep0_io(dev, &io, data, true);
+	kfree(data);
+	return ret;
+}
+
+static int raw_ioctl_ep0_read(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	void *data;
+	struct usb_raw_ep_io io;
+	unsigned int length;
+
+	data = raw_alloc_io_data(&io, (void __user *)value, false);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+	ret = raw_process_ep0_io(dev, &io, data, false);
+	if (ret < 0) {
+		kfree(data);
+		return ret;
+	}
+	length = min(io.length, (unsigned int)ret);
+	ret = copy_to_user((void __user *)(value + sizeof(io)), data, length);
+	kfree(data);
+	return ret;
+}
+
+static bool check_ep_caps(struct usb_ep *ep,
+				struct usb_endpoint_descriptor *desc)
+{
+	switch (usb_endpoint_type(desc)) {
+	case USB_ENDPOINT_XFER_ISOC:
+		if (!ep->caps.type_iso)
+			return false;
+		break;
+	case USB_ENDPOINT_XFER_BULK:
+		if (!ep->caps.type_bulk)
+			return false;
+		break;
+	case USB_ENDPOINT_XFER_INT:
+		if (!ep->caps.type_int)
+			return false;
+		break;
+	default:
+		return false;
+	}
+
+	if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
+		return false;
+	if (usb_endpoint_dir_out(desc) && !ep->caps.dir_out)
+		return false;
+
+	return true;
+}
+
+static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0, i;
+	unsigned long flags;
+	struct usb_endpoint_descriptor *desc;
+	struct usb_ep *ep = NULL;
+
+	desc = memdup_user((void __user *)value, sizeof(*desc));
+	if (IS_ERR(desc))
+		return PTR_ERR(desc);
+
+	/*
+	 * Endpoints with a maxpacket length of 0 can cause crashes in UDC
+	 * drivers.
+	 */
+	if (usb_endpoint_maxp(desc) == 0) {
+		kfree(desc);
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_free;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_free;
+	}
+
+	for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
+		if (dev->eps[i].state == STATE_EP_ENABLED)
+			continue;
+		break;
+	}
+	if (i == USB_RAW_MAX_ENDPOINTS) {
+		pr_debug("fail, no device endpoints available\n");
+		ret = -EBUSY;
+		goto out_free;
+	}
+
+	gadget_for_each_ep(ep, dev->gadget) {
+		if (ep->enabled)
+			continue;
+		if (!check_ep_caps(ep, desc))
+			continue;
+		ep->desc = desc;
+		ret = usb_ep_enable(ep);
+		if (ret < 0) {
+			pr_err("fail, usb_ep_enable returned %d\n", ret);
+			goto out_free;
+		}
+		dev->eps[i].req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+		if (!dev->eps[i].req) {
+			pr_err("fail, usb_ep_alloc_request failed\n");
+			usb_ep_disable(ep);
+			ret = -ENOMEM;
+			goto out_free;
+		}
+		dev->eps[i].ep = ep;
+		dev->eps[i].state = STATE_EP_ENABLED;
+		ep->driver_data = &dev->eps[i];
+		ret = i;
+		goto out_unlock;
+	}
+
+	pr_debug("fail, no gadget endpoints available\n");
+	ret = -EBUSY;
+
+out_free:
+	kfree(desc);
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0, i = value;
+	unsigned long flags;
+	const void *desc;
+
+	if (i < 0 || i >= USB_RAW_MAX_ENDPOINTS)
+		return -EINVAL;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (dev->eps[i].state != STATE_EP_ENABLED) {
+		pr_debug("fail, endpoint is not enabled\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (dev->eps[i].disabling) {
+		pr_debug("fail, disable already in progress\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	dev->eps[i].disabling = true;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	usb_ep_disable(dev->eps[i].ep);
+
+	spin_lock_irqsave(&dev->lock, flags);
+	usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
+	desc = dev->eps[i].ep->desc;
+	dev->eps[i].ep = NULL;
+	dev->eps[i].state = STATE_EP_DISABLED;
+	kfree(desc);
+	dev->eps[i].disabling = false;
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static void gadget_ep_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct raw_ep *r_ep = (struct raw_ep *)ep->driver_data;
+	struct raw_dev *dev = r_ep->dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (req->status)
+		r_ep->status = req->status;
+	else
+		r_ep->status = req->actual;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	complete((struct completion *)req->context);
+}
+
+static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
+				void *data, bool in)
+{
+	int ret = 0;
+	unsigned long flags;
+	struct raw_ep *ep = &dev->eps[io->ep];
+	DECLARE_COMPLETION_ONSTACK(done);
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (ep->state != STATE_EP_ENABLED) {
+		pr_debug("fail, endpoint is not enabled\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (ep->disabling) {
+		pr_debug("fail, endpoint is being disabled\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if (ep->urb_queued) {
+		pr_debug("fail, urb already queued\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
+		pr_debug("fail, wrong direction\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	ep->dev = dev;
+	ep->req->context = &done;
+	ep->req->complete = gadget_ep_complete;
+	ep->req->buf = data;
+	ep->req->length = io->length;
+	ep->req->zero = usb_raw_io_flags_zero(io->flags);
+	ep->urb_queued = true;
+	spin_unlock_irqrestore(&dev->lock, flags);
+
+	ret = usb_ep_queue(ep->ep, ep->req, GFP_ATOMIC);
+	if (ret) {
+		pr_err("fail, usb_ep_queue returned %d\n", ret);
+		spin_lock_irqsave(&dev->lock, flags);
+		dev->state = STATE_DEV_FAILED;
+		goto out_done;
+	}
+
+	ret = wait_for_completion_interruptible(&done);
+	if (ret) {
+		pr_debug("wait interrupted\n");
+		usb_ep_dequeue(ep->ep, ep->req);
+		wait_for_completion(&done);
+		spin_lock_irqsave(&dev->lock, flags);
+		goto out_done;
+	}
+
+	spin_lock_irqsave(&dev->lock, flags);
+	ret = ep->status;
+
+out_done:
+	ep->urb_queued = false;
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_ep_write(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	char *data;
+	struct usb_raw_ep_io io;
+
+	data = raw_alloc_io_data(&io, (void __user *)value, true);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+	ret = raw_process_ep_io(dev, &io, data, true);
+	kfree(data);
+	return ret;
+}
+
+static int raw_ioctl_ep_read(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	char *data;
+	struct usb_raw_ep_io io;
+	unsigned int length;
+
+	data = raw_alloc_io_data(&io, (void __user *)value, false);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+	ret = raw_process_ep_io(dev, &io, data, false);
+	if (ret < 0) {
+		kfree(data);
+		return ret;
+	}
+	length = min(io.length, (unsigned int)ret);
+	ret = copy_to_user((void __user *)(value + sizeof(io)), data, length);
+	kfree(data);
+	return ret;
+}
+
+static int raw_ioctl_configure(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	if (value)
+		return -EINVAL;
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	usb_gadget_set_state(dev->gadget, USB_STATE_CONFIGURED);
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static int raw_ioctl_vbus_draw(struct raw_dev *dev, unsigned long value)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	if (dev->state != STATE_DEV_RUNNING) {
+		pr_debug("fail, device is not running\n");
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+	if (!dev->gadget) {
+		pr_debug("fail, gadget is not bound\n");
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+	usb_gadget_vbus_draw(dev->gadget, 2 * value);
+
+out_unlock:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return ret;
+}
+
+static long raw_ioctl(struct file *fd, unsigned int cmd, unsigned long value)
+{
+	struct raw_dev *dev = fd->private_data;
+	int ret = 0;
+
+	if (!dev)
+		return -EBUSY;
+
+	switch (cmd) {
+	case USB_RAW_IOCTL_INIT:
+		ret = raw_ioctl_init(dev, value);
+		break;
+	case USB_RAW_IOCTL_RUN:
+		ret = raw_ioctl_run(dev, value);
+		break;
+	case USB_RAW_IOCTL_EVENT_FETCH:
+		ret = raw_ioctl_event_fetch(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP0_WRITE:
+		ret = raw_ioctl_ep0_write(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP0_READ:
+		ret = raw_ioctl_ep0_read(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP_ENABLE:
+		ret = raw_ioctl_ep_enable(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP_DISABLE:
+		ret = raw_ioctl_ep_disable(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP_WRITE:
+		ret = raw_ioctl_ep_write(dev, value);
+		break;
+	case USB_RAW_IOCTL_EP_READ:
+		ret = raw_ioctl_ep_read(dev, value);
+		break;
+	case USB_RAW_IOCTL_CONFIGURE:
+		ret = raw_ioctl_configure(dev, value);
+		break;
+	case USB_RAW_IOCTL_VBUS_DRAW:
+		ret = raw_ioctl_vbus_draw(dev, value);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+/*----------------------------------------------------------------------*/
+
+static const struct file_operations raw_fops = {

drivers/usb/gadget/legacy/raw.c line 1048

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	.open =			raw_open,
+	.unlocked_ioctl =	raw_ioctl,
+	.compat_ioctl =		raw_ioctl,
+	.release =		raw_release,
+	.llseek =		no_llseek,
+};
+
+static struct miscdevice raw_device = {
+	.minor = MISC_DYNAMIC_MINOR,
+	.name = "raw-gadget",

DRIVER_NAME?

Quoted 12 lines
drivers/usb/gadget/legacy/raw.c new file +12 -0
+	.fops = &raw_fops,
+};
+
+static int __init raw_init(void)
+{
+	int rv;
+
+	rv = misc_register(&raw_device);
+	if (rv) {
+		pr_err("failed to register raw-gadget device with %d\n", rv);
+		return rv;
+	}

drivers/usb/gadget/legacy/raw.c line 1070

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+	return 0;
+}
+
+static void __exit raw_exit(void)
+{
+	misc_deregister(&raw_device);
+}
+
+module_init(raw_init);
+module_exit(raw_exit);

module_misc_device()? :)

Looks good.

thanks,

greg k-h

#3 lore raw
Quoted 24 lines · Greg Kroah-Hartman
Quoted 22 lines · Andrey Konovalov

USB Raw Gadget is a kernel module that provides a userspace interface for the USB Gadget subsystem. Essentially it allows to emulate USB devices from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is currently a strictly debugging feature and shouldn't be used in production.

Raw Gadget is similar to GadgetFS, but provides a more low-level and direct access to the USB Gadget layer for the userspace. The key differences are:

1. Every USB request is passed to the userspace to get a response, while GadgetFS responds to some USB requests internally based on the provided descriptors. However note, that the UDC driver might respond to some requests on its own and never forward them to the Gadget layer.

2. GadgetFS performs some sanity checks on the provided USB descriptors, while Raw Gadget allows you to provide arbitrary data as responses to USB requests.

3. Raw Gadget provides a way to select a UDC device/driver to bind to, while GadgetFS currently binds to the first available UDC.

Quoted 7 lines
Quoted 5 lines

4. Raw Gadget uses predictable endpoint names (handles) across different UDCs (as long as UDCs have enough endpoints of each required transfer type).

5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.

Looks good to me, only minor comments below.

Great, thanks!

About reworking the logging to use dev_err/dbg(): can I pass the global miscdevice struct into those macros? Or should I pass a pointer to this struct into all of the functions that print log messages? The latter seems unnecessarily complex, unless there's a reason to do that.

For the rest of the comments please see below.

Quoted 97 lines
Quoted 97 lines
---
 Documentation/usb/index.rst         |    1 +
 Documentation/usb/raw-gadget.rst    |   59 ++
 drivers/usb/gadget/legacy/Kconfig   |    8 +
 drivers/usb/gadget/legacy/Makefile  |    1 +
 drivers/usb/gadget/legacy/raw.c     | 1070 +++++++++++++++++++++++++++
 include/uapi/linux/usb/raw_gadget.h |  167 +++++
 6 files changed, 1306 insertions(+)
 create mode 100644 Documentation/usb/raw-gadget.rst
 create mode 100644 drivers/usb/gadget/legacy/raw.c
 create mode 100644 include/uapi/linux/usb/raw_gadget.h
Documentation/usb/index.rst +1 -0
index e55386a4abfb..90310e2a0c1f 100644
@@ -22,6 +22,7 @@ USB support
     misc_usbsevseg
     mtouchusb
     ohci
+    raw-gadget
     rio
     usbip_protocol
     usbmon
Documentation/usb/raw-gadget.rst new file +59 -0
new file mode 100644
index 000000000000..cbedf5451ed3
@@ -0,0 +1,59 @@
+==============
+USB Raw Gadget
+==============
+
+USB Raw Gadget is a kernel module that provides a userspace interface for
+the USB Gadget subsystem. Essentially it allows to emulate USB devices
+from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is
+currently a strictly debugging feature and shouldn't be used in
+production, use GadgetFS instead.
+
+Comparison to GadgetFS
+~~~~~~~~~~~~~~~~~~~~~~
+
+Raw Gadget is similar to GadgetFS, but provides a more low-level and
+direct access to the USB Gadget layer for the userspace. The key
+differences are:
+
+1. Every USB request is passed to the userspace to get a response, while
+   GadgetFS responds to some USB requests internally based on the provided
+   descriptors. However note, that the UDC driver might respond to some
+   requests on its own and never forward them to the Gadget layer.
+
+2. GadgetFS performs some sanity checks on the provided USB descriptors,
+   while Raw Gadget allows you to provide arbitrary data as responses to
+   USB requests.
+
+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).
+
+5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.
+
+Userspace interface
+~~~~~~~~~~~~~~~~~~~
+
+To create a Raw Gadget instance open /dev/raw-gadget. Multiple raw-gadget
+instances (bound to different UDCs) can be used at the same time. The
+interaction with the opened file happens through the ioctl() calls, see
+comments in include/uapi/linux/usb/raw_gadget.h for details.
+
+The typical usage of Raw Gadget looks like:
+
+1. Open Raw Gadget instance via /dev/raw-gadget.
+2. Initialize the instance via USB_RAW_IOCTL_INIT.
+3. Launch the instance with USB_RAW_IOCTL_RUN.
+4. In a loop issue USB_RAW_IOCTL_EVENT_FETCH calls to receive events from
+   Raw Gadget and react to those depending on what kind of USB device
+   needs to be emulated.
+
+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.
drivers/usb/gadget/legacy/Kconfig +0 -0
index 119a4e47681f..8c947edf7515 100644
@@ -489,3 +489,11 @@ config USB_G_WEBCAM
 
Quoted 12 lines

drivers/usb/gadget/legacy/Kconfig config USB_G_WEBCAM · line 499

Quoted 10 lines
drivers/usb/gadget/legacy/Kconfig +8 -0
        Say "y" to link the driver statically, or "m" to build a
        dynamically linked module called "g_webcam".
+
+config USB_RAW_GADGET
+     tristate "USB Raw Gadget"
+     help
+       USB Raw Gadget is a kernel module that provides a userspace interface
+       for the USB Gadget subsystem. Essentially it allows to emulate USB
+       devices from userspace. See Documentation/usb/raw-gadget.rst for
+       details.

No help text saying what the module name would be if selected as one?

Will add in v4.

Quoted 15 lines

drivers/usb/gadget/legacy/Makefile obj-$(CONFIG_USB_G_WEBCAM) += g_webcam.o · line 46

Quoted 11 lines
drivers/usb/gadget/legacy/Makefile +1 -0
index abd0c3e66a05..799feefeee58 100644
@@ -43,3 +43,4 @@ obj-$(CONFIG_USB_G_WEBCAM)  += g_webcam.o
 obj-$(CONFIG_USB_G_NCM)              += g_ncm.o
 obj-$(CONFIG_USB_G_ACM_MS)   += g_acm_ms.o
 obj-$(CONFIG_USB_GADGET_TARGET)      += tcm_usb_gadget.o
+obj-$(CONFIG_USB_RAW_GADGET) += raw.o

raw.ko? I think we already have a raw.ko in the kernel, shouldn't this be usb_raw_gadget.ko or something?

Quoted 2 lines

Yeah, we do, look at drivers/char/raw.c. This needs to be changed, I can't take it as-is otherwise the builders will croak on it.

OK, I think raw_gadget.ko is short enough and unique. Will change it in in v4.

Quoted 39 lines
Quoted 7 lines
drivers/usb/gadget/legacy/raw.c new file +0 -0
new file mode 100644
index 000000000000..fcfa2ebc103e

drivers/usb/gadget/legacy/raw.c line 9

Quoted 9 lines
drivers/usb/gadget/legacy/raw.c new file +9 -0
@@ -0,0 +1,1070 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Raw Gadget driver.
+ * See Documentation/usb/raw-gadget.rst for more details.
+ *
+ * Andrey Konovalov <[email protected]>
+ */
+
+#define pr_fmt(fmt) "raw: %s: " fmt, __func__

"raw gadget"?

This a driver, why do you ever need to do a pr_* call and not a dev_* instead?

Quoted 17 lines
drivers/usb/gadget/legacy/raw.c new file +17 -0
+
+#include <linux/compiler.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/kref.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/semaphore.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/wait.h>
+
+#include <linux/usb.h>
+#include <linux/usb/ch9.h>
+#include <linux/usb/ch11.h>
+#include <linux/usb/gadget.h>
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 31

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+
+#include <uapi/linux/usb/raw_gadget.h>
+
+#define      DRIVER_DESC "USB Raw Gadget"
+#define DRIVER_NAME "raw-gadget"

You only use this in one place. You can use it in the definition of the misc driver name also, right?

Yes, will do in v4.

Quoted 17 lines
Quoted 4 lines
drivers/usb/gadget/legacy/raw.c new file +2 -0
+
+MODULE_DESCRIPTION(DRIVER_DESC);

drivers/usb/gadget/legacy/raw.c line 43

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+MODULE_AUTHOR("Andrey Konovalov");
+MODULE_LICENSE("GPL");
+
+/*----------------------------------------------------------------------*/
+
+#define RAW_EVENT_QUEUE_SIZE 128
+
+struct raw_event_queue {
+     spinlock_t              lock;
+     struct semaphore        sema;

Two locks for the same structure? That feels "wrong". Why is this? document the heck out of it if it is required please.

raw_event_queue_fetch() can be called concurrently, so there we first check that there's at least one event queued by decrementing the semaphore, and then take the lock to protect queue struct fields. Does this make sense?

I'll add comment explaining that in v4.

Quoted 15 lines
Quoted 15 lines
drivers/usb/gadget/legacy/raw.c new file +14 -0
+     struct usb_raw_event    *events[RAW_EVENT_QUEUE_SIZE];
+     int                     size;
+};
+
+static void raw_event_queue_init(struct raw_event_queue *queue)
+{
+     spin_lock_init(&queue->lock);
+     sema_init(&queue->sema, 0);
+     queue->size = 0;
+}
+
+static int raw_event_queue_add(struct raw_event_queue *queue,
+     enum usb_raw_event_type type, size_t length, const void *data)
+{
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 63

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+     unsigned long flags;
+     struct usb_raw_event *event;
+
+     spin_lock_irqsave(&queue->lock, flags);
+     if (queue->size >= RAW_EVENT_QUEUE_SIZE) {
+             pr_err("too many events\n");

What can someone do with this error?

Hm, I guess we better change this to WARN_ON(), since if this is triggered, there's probably a flaw in raw gadget that we want to detect and get fixed.

Quoted 25 lines

dev_err() to show exactly where it came from?

Quoted 22 lines
drivers/usb/gadget/legacy/raw.c new file +22 -0
+             spin_unlock_irqrestore(&queue->lock, flags);
+             return -ENOMEM;
+     }
+     event = kmalloc(sizeof(*event) + length, GFP_ATOMIC);
+     if (!event) {
+             spin_unlock_irqrestore(&queue->lock, flags);
+             return -ENOMEM;
+     }
+     event->type = type;
+     event->length = length;
+     if (event->length)
+             memcpy(&event->data[0], data, length);
+     queue->events[queue->size] = event;
+     queue->size++;
+     up(&queue->sema);
+     spin_unlock_irqrestore(&queue->lock, flags);
+     return 0;
+}
+
+static struct usb_raw_event *raw_event_queue_fetch(
+                             struct raw_event_queue *queue)
+{
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 91

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+     unsigned long flags;
+     struct usb_raw_event *event;
+
+     if (down_interruptible(&queue->sema))
+             return NULL;
+     spin_lock_irqsave(&queue->lock, flags);

You just locked twice?

See the explanation above.

Quoted 7 lines

drivers/usb/gadget/legacy/raw.c line 93

Quoted 3 lines
drivers/usb/gadget/legacy/raw.c new file +2 -0
+     if (WARN_ON(!queue->size))
+             return NULL;

It's as if you want to trigger syzbot :)

Just log it and return an error.

We do want to trigger syzbot here =) Same as with the upper bound, if this is hit, there's probably a flaw in raw gadget that we want to detect and get fixed.

Quoted 10 lines
Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +9 -0
+     event = queue->events[0];
+     queue->size--;
+     memmove(&queue->events[0], &queue->events[1],
+                     queue->size * sizeof(queue->events[0]));
+     spin_unlock_irqrestore(&queue->lock, flags);
+     return event;
+}
+
+static void raw_event_queue_destroy(struct raw_event_queue *queue)
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 108

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+{
+     int i;
+
+     for (i = 0; i < queue->size; i++)
+             kfree(queue->events[i]);
+     queue->size = 0;

No locking needed?

No. This is called from dev_free(), which is only called when all references go away, so there are no concurrent accesses.

Quoted 42 lines
Quoted 29 lines
drivers/usb/gadget/legacy/raw.c new file +28 -0
+}
+
+/*----------------------------------------------------------------------*/
+
+struct raw_dev;
+
+#define USB_RAW_MAX_ENDPOINTS 32
+
+enum ep_state {
+     STATE_EP_DISABLED,
+     STATE_EP_ENABLED,
+};
+
+struct raw_ep {
+     struct raw_dev          *dev;
+     enum ep_state           state;
+     struct usb_ep           *ep;
+     struct usb_request      *req;
+     bool                    urb_queued;
+     bool                    disabling;
+     ssize_t                 status;
+};
+
+enum dev_state {
+     STATE_DEV_INVALID = 0,
+     STATE_DEV_OPENED,
+     STATE_DEV_INITIALIZED,
+     STATE_DEV_RUNNING,

drivers/usb/gadget/legacy/raw.c line 146

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+     STATE_DEV_CLOSED,
+     STATE_DEV_FAILED
+};
+
+struct raw_dev {
+     struct kref                     count;
+     spinlock_t                      lock;
+
+     const char                      *udc_name;
+     struct usb_gadget_driver        driver;

A dev embeds a driver?

Quoted 4 lines

Not a pointer?

But you have a kref, so the reference count of this object is there, right?

I didn't get this comment, could you elaborate? I can make it a pointer, but for each raw_dev we have a unique usb_gadget_driver instance, so embedding it as is is simpler.

Quoted 136 lines
Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +4 -0
+
+     /* Protected by lock: */
+     enum dev_state                  state;
+     bool                            gadget_registered;

drivers/usb/gadget/legacy/raw.c line 160

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+     struct usb_gadget               *gadget;
+     struct usb_request              *req;
+     bool                            ep0_in_pending;
+     bool                            ep0_out_pending;
+     bool                            ep0_urb_queued;
+     ssize_t                         ep0_status;
+     struct raw_ep                   eps[USB_RAW_MAX_ENDPOINTS];
+
+     struct completion               ep0_done;
+     struct raw_event_queue          queue;

No pointer to your misc device? I think you need that for logging.

Quoted 118 lines
drivers/usb/gadget/legacy/raw.c new file +118 -0
+};
+
+static struct raw_dev *dev_new(void)
+{
+     struct raw_dev *dev;
+
+     dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+     if (!dev)
+             return NULL;
+     /* Matches kref_put() in raw_release(). */
+     kref_init(&dev->count);
+     spin_lock_init(&dev->lock);
+     init_completion(&dev->ep0_done);
+     raw_event_queue_init(&dev->queue);
+     return dev;
+}
+
+static void dev_free(struct kref *kref)
+{
+     struct raw_dev *dev = container_of(kref, struct raw_dev, count);
+     int i;
+
+     kfree(dev->udc_name);
+     kfree(dev->driver.udc_name);
+     if (dev->req) {
+             if (dev->ep0_urb_queued)
+                     usb_ep_dequeue(dev->gadget->ep0, dev->req);
+             usb_ep_free_request(dev->gadget->ep0, dev->req);
+     }
+     raw_event_queue_destroy(&dev->queue);
+     for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
+             if (dev->eps[i].state != STATE_EP_ENABLED)
+                     continue;
+             usb_ep_disable(dev->eps[i].ep);
+             usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
+             kfree(dev->eps[i].ep->desc);
+             dev->eps[i].state = STATE_EP_DISABLED;
+     }
+     kfree(dev);
+}
+
+/*----------------------------------------------------------------------*/
+
+static int raw_queue_event(struct raw_dev *dev,
+     enum usb_raw_event_type type, size_t length, const void *data)
+{
+     int ret = 0;
+     unsigned long flags;
+
+     ret = raw_event_queue_add(&dev->queue, type, length, data);
+     if (ret < 0) {
+             spin_lock_irqsave(&dev->lock, flags);
+             dev->state = STATE_DEV_FAILED;
+             spin_unlock_irqrestore(&dev->lock, flags);
+     }
+     return ret;
+}
+
+static void gadget_ep0_complete(struct usb_ep *ep, struct usb_request *req)
+{
+     struct raw_dev *dev = req->context;
+     unsigned long flags;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (req->status)
+             dev->ep0_status = req->status;
+     else
+             dev->ep0_status = req->actual;
+     if (dev->ep0_in_pending)
+             dev->ep0_in_pending = false;
+     else
+             dev->ep0_out_pending = false;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     complete(&dev->ep0_done);
+}
+
+static int gadget_bind(struct usb_gadget *gadget,
+                     struct usb_gadget_driver *driver)
+{
+     int ret = 0;
+     struct raw_dev *dev = container_of(driver, struct raw_dev, driver);
+     struct usb_request *req;
+     unsigned long flags;
+
+     if (strcmp(gadget->name, dev->udc_name) != 0)
+             return -ENODEV;
+
+     set_gadget_data(gadget, dev);
+     req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
+     if (!req) {
+             pr_err("usb_ep_alloc_request failed\n");
+             set_gadget_data(gadget, NULL);
+             return -ENOMEM;
+     }
+
+     spin_lock_irqsave(&dev->lock, flags);
+     dev->req = req;
+     dev->req->context = dev;
+     dev->req->complete = gadget_ep0_complete;
+     dev->gadget = gadget;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     /* Matches kref_put() in gadget_unbind(). */
+     kref_get(&dev->count);
+
+     ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL);
+     if (ret < 0)
+             pr_err("failed to queue event\n");
+
+     return ret;
+}
+
+static void gadget_unbind(struct usb_gadget *gadget)
+{
+     struct raw_dev *dev = get_gadget_data(gadget);
+     unsigned long flags;
+
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 283

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+     spin_lock_irqsave(&dev->lock, flags);
+     set_gadget_data(gadget, NULL);
+     spin_unlock_irqrestore(&dev->lock, flags);
+     /* Matches kref_get() in gadget_bind(). */
+     kref_put(&dev->count, dev_free);

What protects the kref from being called 'put' twice on the same pointer at the same time? There should be some lock somewhere, right?

Hm, kref_put() does refcount_dec_and_test(), which in turns calls atomic_dec_and_test(), so this is protected against concurrent puts (which is the whole idea of kref?), and no locking is needed. Unless I misunderstand something.

Quoted 79 lines
Quoted 44 lines
drivers/usb/gadget/legacy/raw.c new file +42 -0
+}
+
+static int gadget_setup(struct usb_gadget *gadget,
+                     const struct usb_ctrlrequest *ctrl)
+{
+     int ret = 0;
+     struct raw_dev *dev = get_gadget_data(gadget);
+     unsigned long flags;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_err("ignoring, device is not running\n");
+             ret = -ENODEV;
+             goto out_unlock;
+     }
+     if (dev->ep0_in_pending || dev->ep0_out_pending) {
+             pr_debug("stalling, already have pending request\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if ((ctrl->bRequestType & USB_DIR_IN) && ctrl->wLength)
+             dev->ep0_in_pending = true;
+     else
+             dev->ep0_out_pending = true;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     ret = raw_queue_event(dev, USB_RAW_EVENT_CONTROL, sizeof(*ctrl), ctrl);
+     if (ret < 0)
+             pr_err("failed to queue event\n");
+     goto out;
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+out:
+     return ret;
+}
+
+/* These are currently ununsed but present in case UDC driver requires them. */
+static void gadget_disconnect(struct usb_gadget *gadget) { }
+static void gadget_suspend(struct usb_gadget *gadget) { }
+static void gadget_resume(struct usb_gadget *gadget) { }
+static void gadget_reset(struct usb_gadget *gadget) { }

drivers/usb/gadget/legacy/raw.c line 335

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+
+/*----------------------------------------------------------------------*/
+
+static int raw_open(struct inode *inode, struct file *fd)
+{
+     struct raw_dev *dev;
+
+     dev = dev_new();
+     if (!dev) {
+             pr_err("failed to created device");

You should have access to your misc device here, so dev_err()?

Quoted 22 lines
drivers/usb/gadget/legacy/raw.c new file +22 -0
+             return -ENOMEM;
+     }
+     fd->private_data = dev;
+     dev->state = STATE_DEV_OPENED;
+     return 0;
+}
+
+static int raw_release(struct inode *inode, struct file *fd)
+{
+     int ret = 0;
+     struct raw_dev *dev = fd->private_data;
+     unsigned long flags;
+     bool unregister = false;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     dev->state = STATE_DEV_CLOSED;
+     if (!dev->gadget) {
+             spin_unlock_irqrestore(&dev->lock, flags);
+             goto out_put;
+     }
+     if (dev->gadget_registered)
+             unregister = true;
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 363

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+     dev->gadget_registered = false;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     if (unregister) {
+             ret = usb_gadget_unregister_driver(&dev->driver);
+             WARN_ON(ret != 0);

you want to crash? :)

No reason to crash here I guess :) Will remove in v4.

Quoted 691 lines
Quoted 56 lines
drivers/usb/gadget/legacy/raw.c new file +55 -0
+             /* Matches kref_get() in raw_ioctl_run(). */
+             kref_put(&dev->count, dev_free);
+     }
+
+out_put:
+     /* Matches dev_new() in raw_open(). */
+     kref_put(&dev->count, dev_free);
+     return ret;
+}
+
+/*----------------------------------------------------------------------*/
+
+static int raw_ioctl_init(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     struct usb_raw_init arg;
+     char *udc_driver_name;
+     char *udc_device_name;
+     unsigned long flags;
+
+     ret = copy_from_user(&arg, (void __user *)value, sizeof(arg));
+     if (ret)
+             return ret;
+
+     switch (arg.speed) {
+     case USB_SPEED_UNKNOWN:
+             arg.speed = USB_SPEED_HIGH;
+             break;
+     case USB_SPEED_LOW:
+     case USB_SPEED_FULL:
+     case USB_SPEED_HIGH:
+     case USB_SPEED_SUPER:
+             break;
+     default:
+             return -EINVAL;
+     }
+
+     udc_driver_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
+     if (!udc_driver_name)
+             return -ENOMEM;
+     ret = strscpy(udc_driver_name, &arg.driver_name[0],
+                             UDC_NAME_LENGTH_MAX);
+     if (ret < 0) {
+             kfree(udc_driver_name);
+             return ret;
+     }
+     ret = 0;
+
+     udc_device_name = kmalloc(UDC_NAME_LENGTH_MAX, GFP_KERNEL);
+     if (!udc_device_name) {
+             kfree(udc_driver_name);
+             return -ENOMEM;
+     }
+     ret = strscpy(udc_device_name, &arg.device_name[0],
+                             UDC_NAME_LENGTH_MAX);

drivers/usb/gadget/legacy/raw.c line 428

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+     if (ret < 0) {
+             kfree(udc_driver_name);
+             kfree(udc_device_name);
+             return ret;
+     }
+     ret = 0;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_OPENED) {
+             pr_debug("fail, device is not opened\n");

dev_dbg().

Quoted 28 lines
drivers/usb/gadget/legacy/raw.c new file +28 -0
+             kfree(udc_driver_name);
+             kfree(udc_device_name);
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     dev->udc_name = udc_driver_name;
+
+     dev->driver.function = DRIVER_DESC;
+     dev->driver.max_speed = arg.speed;
+     dev->driver.setup = gadget_setup;
+     dev->driver.disconnect = gadget_disconnect;
+     dev->driver.bind = gadget_bind;
+     dev->driver.unbind = gadget_unbind;
+     dev->driver.suspend = gadget_suspend;
+     dev->driver.resume = gadget_resume;
+     dev->driver.reset = gadget_reset;
+     dev->driver.driver.name = DRIVER_NAME;
+     dev->driver.udc_name = udc_device_name;
+     dev->driver.match_existing_only = 1;
+
+     dev->state = STATE_DEV_INITIALIZED;
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_run(struct raw_dev *dev, unsigned long value)

drivers/usb/gadget/legacy/raw.c line 466

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+{
+     int ret = 0;
+     unsigned long flags;
+
+     if (value)
+             return -EINVAL;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_INITIALIZED) {
+             pr_debug("fail, device is not initialized\n");

dev_dbg()

drivers/usb/gadget/legacy/raw.c line 476

Quoted 10 lines
drivers/usb/gadget/legacy/raw.c new file +10 -0
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     ret = usb_gadget_probe_driver(&dev->driver);
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (ret) {
+             pr_err("fail, usb_gadget_probe_driver returned %d\n", ret);

dev_err()

same goes for everywhere, no need for any pr_* calls in this driver.

Quoted 566 lines
drivers/usb/gadget/legacy/raw.c new file +566 -0
+             dev->state = STATE_DEV_FAILED;
+             goto out_unlock;
+     }
+     dev->gadget_registered = true;
+     dev->state = STATE_DEV_RUNNING;
+     /* Matches kref_put() in raw_release(). */
+     kref_get(&dev->count);
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_event_fetch(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     struct usb_raw_event arg;
+     unsigned long flags;
+     struct usb_raw_event *event;
+     uint32_t length;
+
+     ret = copy_from_user(&arg, (void __user *)value, sizeof(arg));
+     if (ret)
+             return ret;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             spin_unlock_irqrestore(&dev->lock, flags);
+             return -EINVAL;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             spin_unlock_irqrestore(&dev->lock, flags);
+             return -EBUSY;
+     }
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     event = raw_event_queue_fetch(&dev->queue);
+     if (!event) {
+             pr_debug("event fetching interrupted\n");
+             return -EINTR;
+     }
+     length = min(arg.length, event->length);
+     ret = copy_to_user((void __user *)value, event,
+                             sizeof(*event) + length);
+     return ret;
+}
+
+static void *raw_alloc_io_data(struct usb_raw_ep_io *io, void __user *ptr,
+                             bool get_from_user)
+{
+     int ret;
+     void *data;
+
+     ret = copy_from_user(io, ptr, sizeof(*io));
+     if (ret)
+             return ERR_PTR(ret);
+     if (io->ep >= USB_RAW_MAX_ENDPOINTS)
+             return ERR_PTR(-EINVAL);
+     if (!usb_raw_io_flags_valid(io->flags))
+             return ERR_PTR(-EINVAL);
+     if (io->length > PAGE_SIZE)
+             return ERR_PTR(-EINVAL);
+     if (get_from_user)
+             data = memdup_user(ptr + sizeof(*io), io->length);
+     else {
+             data = kmalloc(io->length, GFP_KERNEL);
+             if (!data)
+                     data = ERR_PTR(-ENOMEM);
+     }
+     return data;
+}
+
+static int raw_process_ep0_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
+                             void *data, bool in)
+{
+     int ret = 0;
+     unsigned long flags;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (dev->ep0_urb_queued) {
+             pr_debug("fail, urb already queued\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if ((in && !dev->ep0_in_pending) ||
+                     (!in && !dev->ep0_out_pending)) {
+             pr_debug("fail, wrong direction\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (WARN_ON(in && dev->ep0_out_pending)) {
+             ret = -ENODEV;
+             dev->state = STATE_DEV_FAILED;
+             goto out_done;
+     }
+     if (WARN_ON(!in && dev->ep0_in_pending)) {
+             ret = -ENODEV;
+             dev->state = STATE_DEV_FAILED;
+             goto out_done;
+     }
+
+     dev->req->buf = data;
+     dev->req->length = io->length;
+     dev->req->zero = usb_raw_io_flags_zero(io->flags);
+     dev->ep0_urb_queued = true;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     ret = usb_ep_queue(dev->gadget->ep0, dev->req, GFP_ATOMIC);
+     if (ret) {
+             pr_err("fail, usb_ep_queue returned %d\n", ret);
+             spin_lock_irqsave(&dev->lock, flags);
+             dev->state = STATE_DEV_FAILED;
+             goto out_done;
+     }
+
+     ret = wait_for_completion_interruptible(&dev->ep0_done);
+     if (ret) {
+             pr_debug("wait interrupted\n");
+             usb_ep_dequeue(dev->gadget->ep0, dev->req);
+             wait_for_completion(&dev->ep0_done);
+             spin_lock_irqsave(&dev->lock, flags);
+             goto out_done;
+     }
+
+     spin_lock_irqsave(&dev->lock, flags);
+     ret = dev->ep0_status;
+
+out_done:
+     dev->ep0_urb_queued = false;
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_ep0_write(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     void *data;
+     struct usb_raw_ep_io io;
+
+     data = raw_alloc_io_data(&io, (void __user *)value, true);
+     if (IS_ERR(data))
+             return PTR_ERR(data);
+     ret = raw_process_ep0_io(dev, &io, data, true);
+     kfree(data);
+     return ret;
+}
+
+static int raw_ioctl_ep0_read(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     void *data;
+     struct usb_raw_ep_io io;
+     unsigned int length;
+
+     data = raw_alloc_io_data(&io, (void __user *)value, false);
+     if (IS_ERR(data))
+             return PTR_ERR(data);
+     ret = raw_process_ep0_io(dev, &io, data, false);
+     if (ret < 0) {
+             kfree(data);
+             return ret;
+     }
+     length = min(io.length, (unsigned int)ret);
+     ret = copy_to_user((void __user *)(value + sizeof(io)), data, length);
+     kfree(data);
+     return ret;
+}
+
+static bool check_ep_caps(struct usb_ep *ep,
+                             struct usb_endpoint_descriptor *desc)
+{
+     switch (usb_endpoint_type(desc)) {
+     case USB_ENDPOINT_XFER_ISOC:
+             if (!ep->caps.type_iso)
+                     return false;
+             break;
+     case USB_ENDPOINT_XFER_BULK:
+             if (!ep->caps.type_bulk)
+                     return false;
+             break;
+     case USB_ENDPOINT_XFER_INT:
+             if (!ep->caps.type_int)
+                     return false;
+             break;
+     default:
+             return false;
+     }
+
+     if (usb_endpoint_dir_in(desc) && !ep->caps.dir_in)
+             return false;
+     if (usb_endpoint_dir_out(desc) && !ep->caps.dir_out)
+             return false;
+
+     return true;
+}
+
+static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0, i;
+     unsigned long flags;
+     struct usb_endpoint_descriptor *desc;
+     struct usb_ep *ep = NULL;
+
+     desc = memdup_user((void __user *)value, sizeof(*desc));
+     if (IS_ERR(desc))
+             return PTR_ERR(desc);
+
+     /*
+      * Endpoints with a maxpacket length of 0 can cause crashes in UDC
+      * drivers.
+      */
+     if (usb_endpoint_maxp(desc) == 0) {
+             kfree(desc);
+             return -EINVAL;
+     }
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_free;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_free;
+     }
+
+     for (i = 0; i < USB_RAW_MAX_ENDPOINTS; i++) {
+             if (dev->eps[i].state == STATE_EP_ENABLED)
+                     continue;
+             break;
+     }
+     if (i == USB_RAW_MAX_ENDPOINTS) {
+             pr_debug("fail, no device endpoints available\n");
+             ret = -EBUSY;
+             goto out_free;
+     }
+
+     gadget_for_each_ep(ep, dev->gadget) {
+             if (ep->enabled)
+                     continue;
+             if (!check_ep_caps(ep, desc))
+                     continue;
+             ep->desc = desc;
+             ret = usb_ep_enable(ep);
+             if (ret < 0) {
+                     pr_err("fail, usb_ep_enable returned %d\n", ret);
+                     goto out_free;
+             }
+             dev->eps[i].req = usb_ep_alloc_request(ep, GFP_ATOMIC);
+             if (!dev->eps[i].req) {
+                     pr_err("fail, usb_ep_alloc_request failed\n");
+                     usb_ep_disable(ep);
+                     ret = -ENOMEM;
+                     goto out_free;
+             }
+             dev->eps[i].ep = ep;
+             dev->eps[i].state = STATE_EP_ENABLED;
+             ep->driver_data = &dev->eps[i];
+             ret = i;
+             goto out_unlock;
+     }
+
+     pr_debug("fail, no gadget endpoints available\n");
+     ret = -EBUSY;
+
+out_free:
+     kfree(desc);
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0, i = value;
+     unsigned long flags;
+     const void *desc;
+
+     if (i < 0 || i >= USB_RAW_MAX_ENDPOINTS)
+             return -EINVAL;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (dev->eps[i].state != STATE_EP_ENABLED) {
+             pr_debug("fail, endpoint is not enabled\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (dev->eps[i].disabling) {
+             pr_debug("fail, disable already in progress\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     dev->eps[i].disabling = true;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     usb_ep_disable(dev->eps[i].ep);
+
+     spin_lock_irqsave(&dev->lock, flags);
+     usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
+     desc = dev->eps[i].ep->desc;
+     dev->eps[i].ep = NULL;
+     dev->eps[i].state = STATE_EP_DISABLED;
+     kfree(desc);
+     dev->eps[i].disabling = false;
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static void gadget_ep_complete(struct usb_ep *ep, struct usb_request *req)
+{
+     struct raw_ep *r_ep = (struct raw_ep *)ep->driver_data;
+     struct raw_dev *dev = r_ep->dev;
+     unsigned long flags;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (req->status)
+             r_ep->status = req->status;
+     else
+             r_ep->status = req->actual;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     complete((struct completion *)req->context);
+}
+
+static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
+                             void *data, bool in)
+{
+     int ret = 0;
+     unsigned long flags;
+     struct raw_ep *ep = &dev->eps[io->ep];
+     DECLARE_COMPLETION_ONSTACK(done);
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (ep->state != STATE_EP_ENABLED) {
+             pr_debug("fail, endpoint is not enabled\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (ep->disabling) {
+             pr_debug("fail, endpoint is being disabled\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if (ep->urb_queued) {
+             pr_debug("fail, urb already queued\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     if ((in && !ep->ep->caps.dir_in) || (!in && ep->ep->caps.dir_in)) {
+             pr_debug("fail, wrong direction\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+
+     ep->dev = dev;
+     ep->req->context = &done;
+     ep->req->complete = gadget_ep_complete;
+     ep->req->buf = data;
+     ep->req->length = io->length;
+     ep->req->zero = usb_raw_io_flags_zero(io->flags);
+     ep->urb_queued = true;
+     spin_unlock_irqrestore(&dev->lock, flags);
+
+     ret = usb_ep_queue(ep->ep, ep->req, GFP_ATOMIC);
+     if (ret) {
+             pr_err("fail, usb_ep_queue returned %d\n", ret);
+             spin_lock_irqsave(&dev->lock, flags);
+             dev->state = STATE_DEV_FAILED;
+             goto out_done;
+     }
+
+     ret = wait_for_completion_interruptible(&done);
+     if (ret) {
+             pr_debug("wait interrupted\n");
+             usb_ep_dequeue(ep->ep, ep->req);
+             wait_for_completion(&done);
+             spin_lock_irqsave(&dev->lock, flags);
+             goto out_done;
+     }
+
+     spin_lock_irqsave(&dev->lock, flags);
+     ret = ep->status;
+
+out_done:
+     ep->urb_queued = false;
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_ep_write(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     char *data;
+     struct usb_raw_ep_io io;
+
+     data = raw_alloc_io_data(&io, (void __user *)value, true);
+     if (IS_ERR(data))
+             return PTR_ERR(data);
+     ret = raw_process_ep_io(dev, &io, data, true);
+     kfree(data);
+     return ret;
+}
+
+static int raw_ioctl_ep_read(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     char *data;
+     struct usb_raw_ep_io io;
+     unsigned int length;
+
+     data = raw_alloc_io_data(&io, (void __user *)value, false);
+     if (IS_ERR(data))
+             return PTR_ERR(data);
+     ret = raw_process_ep_io(dev, &io, data, false);
+     if (ret < 0) {
+             kfree(data);
+             return ret;
+     }
+     length = min(io.length, (unsigned int)ret);
+     ret = copy_to_user((void __user *)(value + sizeof(io)), data, length);
+     kfree(data);
+     return ret;
+}
+
+static int raw_ioctl_configure(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     unsigned long flags;
+
+     if (value)
+             return -EINVAL;
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     usb_gadget_set_state(dev->gadget, USB_STATE_CONFIGURED);
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static int raw_ioctl_vbus_draw(struct raw_dev *dev, unsigned long value)
+{
+     int ret = 0;
+     unsigned long flags;
+
+     spin_lock_irqsave(&dev->lock, flags);
+     if (dev->state != STATE_DEV_RUNNING) {
+             pr_debug("fail, device is not running\n");
+             ret = -EINVAL;
+             goto out_unlock;
+     }
+     if (!dev->gadget) {
+             pr_debug("fail, gadget is not bound\n");
+             ret = -EBUSY;
+             goto out_unlock;
+     }
+     usb_gadget_vbus_draw(dev->gadget, 2 * value);
+
+out_unlock:
+     spin_unlock_irqrestore(&dev->lock, flags);
+     return ret;
+}
+
+static long raw_ioctl(struct file *fd, unsigned int cmd, unsigned long value)
+{
+     struct raw_dev *dev = fd->private_data;
+     int ret = 0;
+
+     if (!dev)
+             return -EBUSY;
+
+     switch (cmd) {
+     case USB_RAW_IOCTL_INIT:
+             ret = raw_ioctl_init(dev, value);
+             break;
+     case USB_RAW_IOCTL_RUN:
+             ret = raw_ioctl_run(dev, value);
+             break;
+     case USB_RAW_IOCTL_EVENT_FETCH:
+             ret = raw_ioctl_event_fetch(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP0_WRITE:
+             ret = raw_ioctl_ep0_write(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP0_READ:
+             ret = raw_ioctl_ep0_read(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP_ENABLE:
+             ret = raw_ioctl_ep_enable(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP_DISABLE:
+             ret = raw_ioctl_ep_disable(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP_WRITE:
+             ret = raw_ioctl_ep_write(dev, value);
+             break;
+     case USB_RAW_IOCTL_EP_READ:
+             ret = raw_ioctl_ep_read(dev, value);
+             break;
+     case USB_RAW_IOCTL_CONFIGURE:
+             ret = raw_ioctl_configure(dev, value);
+             break;
+     case USB_RAW_IOCTL_VBUS_DRAW:
+             ret = raw_ioctl_vbus_draw(dev, value);
+             break;
+     default:
+             ret = -EINVAL;
+     }
+
+     return ret;
+}
+
+/*----------------------------------------------------------------------*/
+
+static const struct file_operations raw_fops = {
+     .open =                 raw_open,
+     .unlocked_ioctl =       raw_ioctl,
+     .compat_ioctl =         raw_ioctl,
+     .release =              raw_release,
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 1048

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+     .llseek =               no_llseek,
+};
+
+static struct miscdevice raw_device = {
+     .minor = MISC_DYNAMIC_MINOR,
+     .name = "raw-gadget",

DRIVER_NAME?

Will do in v4.

Quoted 17 lines
Quoted 17 lines
drivers/usb/gadget/legacy/raw.c new file +16 -0
+     .fops = &raw_fops,
+};
+
+static int __init raw_init(void)
+{
+     int rv;
+
+     rv = misc_register(&raw_device);
+     if (rv) {
+             pr_err("failed to register raw-gadget device with %d\n", rv);
+             return rv;
+     }
+     return 0;
+}
+
+static void __exit raw_exit(void)
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 1070

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+{
+     misc_deregister(&raw_device);
+}
+
+module_init(raw_init);
+module_exit(raw_exit);

module_misc_device()? :)

Will do in v4.

Thanks!

#4 lore raw
Quoted 34 lines · Andrey Konovalov
Quoted 24 lines · Greg Kroah-Hartman
Quoted 22 lines · Andrey Konovalov

USB Raw Gadget is a kernel module that provides a userspace interface for the USB Gadget subsystem. Essentially it allows to emulate USB devices from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is currently a strictly debugging feature and shouldn't be used in production.

Raw Gadget is similar to GadgetFS, but provides a more low-level and direct access to the USB Gadget layer for the userspace. The key differences are:

1. Every USB request is passed to the userspace to get a response, while GadgetFS responds to some USB requests internally based on the provided descriptors. However note, that the UDC driver might respond to some requests on its own and never forward them to the Gadget layer.

2. GadgetFS performs some sanity checks on the provided USB descriptors, while Raw Gadget allows you to provide arbitrary data as responses to USB requests.

3. Raw Gadget provides a way to select a UDC device/driver to bind to, while GadgetFS currently binds to the first available UDC.

Quoted 7 lines
Quoted 5 lines

4. Raw Gadget uses predictable endpoint names (handles) across different UDCs (as long as UDCs have enough endpoints of each required transfer type).

5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.

Looks good to me, only minor comments below.

Quoted 7 lines

Great, thanks!

About reworking the logging to use dev_err/dbg(): can I pass the global miscdevice struct into those macros? Or should I pass a pointer to this struct into all of the functions that print log messages? The latter seems unnecessarily complex, unless there's a reason to do that.

Ah, you are right, you only have one misc device here. No, that's not good, but you can use it for some messages (your ioctl errors), but ideally you will have a struct device somewhere for each of the "instances" you create, right? That is what you should use for that.

Quoted 9 lines
Quoted 9 lines

drivers/usb/gadget/legacy/raw.c line 146

Quoted 6 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+struct raw_dev {
+     struct kref                     count;
+     spinlock_t                      lock;
+
+     const char                      *udc_name;
+     struct usb_gadget_driver        driver;

A dev embeds a driver?

Quoted 8 lines
Quoted 4 lines

Not a pointer?

But you have a kref, so the reference count of this object is there, right?

I didn't get this comment, could you elaborate? I can make it a pointer, but for each raw_dev we have a unique usb_gadget_driver instance, so embedding it as is is simpler.

Ok, that's fine. But it feels odd creating a driver dynamically to me, but it should work (as you show.) It doesn't give you something to use for the dev_* messages directly, ah, but you do have something:

Quoted 5 lines
Quoted 5 lines

drivers/usb/gadget/legacy/raw.c line 151

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+
+     /* Protected by lock: */
+     enum dev_state                  state;
+     bool                            gadget_registered;
+     struct usb_gadget               *gadget;

There, use that pointer for your dev_* messages, and you should be fine.

Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+static void gadget_unbind(struct usb_gadget *gadget)
+{
+     struct raw_dev *dev = get_gadget_data(gadget);
+     unsigned long flags;
+
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 283

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+     spin_lock_irqsave(&dev->lock, flags);
+     set_gadget_data(gadget, NULL);
+     spin_unlock_irqrestore(&dev->lock, flags);
+     /* Matches kref_get() in gadget_bind(). */
+     kref_put(&dev->count, dev_free);

What protects the kref from being called 'put' twice on the same pointer at the same time? There should be some lock somewhere, right?

Quoted 4 lines

Hm, kref_put() does refcount_dec_and_test(), which in turns calls atomic_dec_and_test(), so this is protected against concurrent puts (which is the whole idea of kref?), and no locking is needed. Unless I misunderstand something.

It's late, but there should be some lock somewhere to prevent a race around this type of thing. That's why we have kref_put_mutex() and kref_put_lock().

Odds are you are fine here, but just something to be aware of...

thanks,

greg k-h

#5 lore raw
Quoted 44 lines · Greg Kroah-Hartman
Quoted 34 lines · Andrey Konovalov
Quoted 24 lines · Greg Kroah-Hartman
Quoted 22 lines · Andrey Konovalov

USB Raw Gadget is a kernel module that provides a userspace interface for the USB Gadget subsystem. Essentially it allows to emulate USB devices from userspace. Enabled with CONFIG_USB_RAW_GADGET. Raw Gadget is currently a strictly debugging feature and shouldn't be used in production.

Raw Gadget is similar to GadgetFS, but provides a more low-level and direct access to the USB Gadget layer for the userspace. The key differences are:

1. Every USB request is passed to the userspace to get a response, while GadgetFS responds to some USB requests internally based on the provided descriptors. However note, that the UDC driver might respond to some requests on its own and never forward them to the Gadget layer.

2. GadgetFS performs some sanity checks on the provided USB descriptors, while Raw Gadget allows you to provide arbitrary data as responses to USB requests.

3. Raw Gadget provides a way to select a UDC device/driver to bind to, while GadgetFS currently binds to the first available UDC.

Quoted 7 lines
Quoted 5 lines

4. Raw Gadget uses predictable endpoint names (handles) across different UDCs (as long as UDCs have enough endpoints of each required transfer type).

5. Raw Gadget has ioctl-based interface instead of a filesystem-based one.

Looks good to me, only minor comments below.

Quoted 7 lines

Great, thanks!

About reworking the logging to use dev_err/dbg(): can I pass the global miscdevice struct into those macros? Or should I pass a pointer to this struct into all of the functions that print log messages? The latter seems unnecessarily complex, unless there's a reason to do that.

Quoted 4 lines

Ah, you are right, you only have one misc device here. No, that's not good, but you can use it for some messages (your ioctl errors), but ideally you will have a struct device somewhere for each of the "instances" you create, right? That is what you should use for that.

OK, I think I got the idea. Will do in v4.

Quoted 50 lines
Quoted 10 lines
Quoted 10 lines

drivers/usb/gadget/legacy/raw.c line 146

Quoted 7 lines
drivers/usb/gadget/legacy/raw.c new file +6 -0
+struct raw_dev {
+     struct kref                     count;
+     spinlock_t                      lock;
+
+     const char                      *udc_name;
+     struct usb_gadget_driver        driver;

A dev embeds a driver?

Quoted 8 lines
Quoted 4 lines

Not a pointer?

But you have a kref, so the reference count of this object is there, right?

I didn't get this comment, could you elaborate? I can make it a pointer, but for each raw_dev we have a unique usb_gadget_driver instance, so embedding it as is is simpler.

Ok, that's fine. But it feels odd creating a driver dynamically to me, but it should work (as you show.) It doesn't give you something to use for the dev_* messages directly, ah, but you do have something:

Quoted 5 lines
Quoted 5 lines

drivers/usb/gadget/legacy/raw.c line 151

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+
+     /* Protected by lock: */
+     enum dev_state                  state;
+     bool                            gadget_registered;
+     struct usb_gadget               *gadget;

There, use that pointer for your dev_* messages, and you should be fine.

Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+static void gadget_unbind(struct usb_gadget *gadget)
+{
+     struct raw_dev *dev = get_gadget_data(gadget);
+     unsigned long flags;
+
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 283

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+     spin_lock_irqsave(&dev->lock, flags);
+     set_gadget_data(gadget, NULL);
+     spin_unlock_irqrestore(&dev->lock, flags);
+     /* Matches kref_get() in gadget_bind(). */
+     kref_put(&dev->count, dev_free);

What protects the kref from being called 'put' twice on the same pointer at the same time? There should be some lock somewhere, right?

Quoted 4 lines

Hm, kref_put() does refcount_dec_and_test(), which in turns calls atomic_dec_and_test(), so this is protected against concurrent puts (which is the whole idea of kref?), and no locking is needed. Unless I misunderstand something.

Quoted 5 lines

It's late, but there should be some lock somewhere to prevent a race around this type of thing. That's why we have kref_put_mutex() and kref_put_lock().

Odds are you are fine here, but just something to be aware of...

Ah, I see. So AFAIU kref_put_lock/mutex() are meant to be used in cases when there might be a concurrent user that doesn't have the reference counter incremented, but holds the lock? We don't do this kind of stuff here.

#6 lore raw
Quoted 23 lines · Andrey Konovalov
Quoted 23 lines
Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+static void gadget_unbind(struct usb_gadget *gadget)
+{
+     struct raw_dev *dev = get_gadget_data(gadget);
+     unsigned long flags;
+
Quoted 8 lines

drivers/usb/gadget/legacy/raw.c line 283

Quoted 5 lines
drivers/usb/gadget/legacy/raw.c new file +5 -0
+     spin_lock_irqsave(&dev->lock, flags);
+     set_gadget_data(gadget, NULL);
+     spin_unlock_irqrestore(&dev->lock, flags);
+     /* Matches kref_get() in gadget_bind(). */
+     kref_put(&dev->count, dev_free);

What protects the kref from being called 'put' twice on the same pointer at the same time? There should be some lock somewhere, right?

Quoted 4 lines

Hm, kref_put() does refcount_dec_and_test(), which in turns calls atomic_dec_and_test(), so this is protected against concurrent puts (which is the whole idea of kref?), and no locking is needed. Unless I misunderstand something.

It's late, but there should be some lock somewhere to prevent a race around this type of thing. That's why we have kref_put_mutex() and kref_put_lock().

Quoted 6 lines
Quoted 1 line

Odds are you are fine here, but just something to be aware of...

Ah, I see. So AFAIU kref_put_lock/mutex() are meant to be used in cases when there might be a concurrent user that doesn't have the reference counter incremented, but holds the lock? We don't do this kind of stuff here.

Ok, as long as there is a lock somewhere preventing this type of thing from happening. Last time I looked at this, it took me and 2 grad students an hour with a whiteboard to work it all out. Which is why the lock variants are there now :)

thanks,

greg k-h