| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 31 Jan 2020 15:42:11 +0200
From: Felipe Balbi <balbi@...nel.org>
To: Andrey Konovalov <andreyknvl@...gle.com>,
linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Jonathan Corbet <corbet@....net>,
Alan Stern <stern@...land.harvard.edu>,
Dmitry Vyukov <dvyukov@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Marco Elver <elver@...gle.com>,
Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH v5 1/1] usb: gadget: add raw-gadget interface
Hi,
Andrey Konovalov <andreyknvl@...gle.com> writes:
> 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.
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Andrey Konovalov <andreyknvl@...gle.com>
> ---
>
> Greg, I've assumed your LGTM meant that I can add a Reviewed-by from you.
>
> Felipe, looking forward to your review, thanks!
>
> Documentation/usb/index.rst | 1 +
> Documentation/usb/raw-gadget.rst | 59 ++
> drivers/usb/gadget/legacy/Kconfig | 11 +
> drivers/usb/gadget/legacy/Makefile | 1 +
> drivers/usb/gadget/legacy/raw_gadget.c | 1068 ++++++++++++++++++++++++
> include/uapi/linux/usb/raw_gadget.h | 167 ++++
> 6 files changed, 1307 insertions(+)
> create mode 100644 Documentation/usb/raw-gadget.rst
> create mode 100644 drivers/usb/gadget/legacy/raw_gadget.c
> create mode 100644 include/uapi/linux/usb/raw_gadget.h
>
> diff --git a/Documentation/usb/index.rst b/Documentation/usb/index.rst
> index e55386a4abfb..90310e2a0c1f 100644
> --- a/Documentation/usb/index.rst
> +++ b/Documentation/usb/index.rst
> @@ -22,6 +22,7 @@ USB support
> misc_usbsevseg
> mtouchusb
> ohci
> + raw-gadget
> rio
> usbip_protocol
> usbmon
> diff --git a/Documentation/usb/raw-gadget.rst b/Documentation/usb/raw-gadget.rst
> new file mode 100644
> index 000000000000..cbedf5451ed3
> --- /dev/null
> +++ b/Documentation/usb/raw-gadget.rst
> @@ -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.
> diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
> index 119a4e47681f..55e495f5d103 100644
> --- a/drivers/usb/gadget/legacy/Kconfig
> +++ b/drivers/usb/gadget/legacy/Kconfig
> @@ -489,3 +489,14 @@ 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.
> +
> + Say "y" to link the driver statically, or "m" to build a
> + dynamically linked module called "raw_gadget".
> diff --git a/drivers/usb/gadget/legacy/Makefile b/drivers/usb/gadget/legacy/Makefile
> index abd0c3e66a05..4d864bf82799 100644
> --- a/drivers/usb/gadget/legacy/Makefile
> +++ b/drivers/usb/gadget/legacy/Makefile
> @@ -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_gadget.o
> diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> new file mode 100644
> index 000000000000..51796af48069
> --- /dev/null
> +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> @@ -0,0 +1,1068 @@
> +// SPDX-License-Identifier: GPL-2.0
V2 only
> +/*
> + * USB Raw Gadget driver.
> + * See Documentation/usb/raw-gadget.rst for more details.
> + *
> + * Andrey Konovalov <andreyknvl@...il.com>
> + */
> +
> +#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>
> +
> +#include <uapi/linux/usb/raw_gadget.h>
> +
> +#define DRIVER_DESC "USB Raw Gadget"
> +#define DRIVER_NAME "raw-gadget"
> +
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_AUTHOR("Andrey Konovalov");
> +MODULE_LICENSE("GPL");
v2+. Care to fix?
> +
> +/*----------------------------------------------------------------------*/
> +
> +#define RAW_EVENT_QUEUE_SIZE 128
> +
> +struct raw_event_queue {
> + /* See the comment in raw_event_queue_fetch() for locking details. */
> + spinlock_t lock;
> + struct semaphore sema;
> + 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)
> +{
> + unsigned long flags;
> + struct usb_raw_event *event;
> +
> + spin_lock_irqsave(&queue->lock, flags);
> + if (WARN_ON(queue->size >= RAW_EVENT_QUEUE_SIZE)) {
> + spin_unlock_irqrestore(&queue->lock, flags);
> + return -ENOMEM;
> + }
> + event = kmalloc(sizeof(*event) + length, GFP_ATOMIC);
I would very much prefer dropping GFP_ATOMIC here. Must you have this
allocation under a spinlock?
--
balbi
Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)
Powered by blists - more mailing lists