* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-01 12:42 Andrej Shadura
2019-04-10 12:02 ` Jiri Kosina
2019-10-03 7:19 ` Andrej Shadura
0 siblings, 2 replies; 15+ messages in thread
From: Andrej Shadura @ 2019-04-01 12:42 UTC (permalink / raw)
To: Jiří Kosina
Cc: linux-input, linux-usb, Andrzej Pietrasiewicz, kernel,
Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
U2F Zero supports custom commands for blinking the LED and getting data
from the internal hardware RNG. Expose the blinking function as a LED
device, and the internal hardware RNG as an HWRNG so that it can be used
to feed the enthropy pool.
Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
---
drivers/hid/Kconfig | 15 ++
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-u2fzero.c | 371 ++++++++++++++++++++++++++++++++++++++
4 files changed, 388 insertions(+)
create mode 100644 drivers/hid/hid-u2fzero.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 41e9935fc584..6efb5a43ffc8 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -989,6 +989,21 @@ config HID_UDRAW_PS3
Say Y here if you want to use the THQ uDraw gaming tablet for
the PS3.
+config HID_U2FZERO
+ tristate "U2F Zero LED and RNG support"
+ depends on USB_HID
+ depends on LEDS_CLASS
+ help
+ Support for the LED of the U2F Zero device.
+
+ U2F Zero supports custom commands for blinking the LED
+ and getting data from the internal hardware RNG.
+ The internal hardware can be used to feed the enthropy pool.
+
+ U2F Zero only supports blinking its LED, so this driver doesn't
+ allow setting the brightness to anything but 1, which will
+ trigger a single blink and immediately reset to back 0.
+
config HID_WACOM
tristate "Wacom Intuos/Graphire tablet support (USB)"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 896a51ce7ce0..d9724ab8df5f 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -108,6 +108,7 @@ obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
obj-$(CONFIG_HID_TIVO) += hid-tivo.o
obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
+obj-$(CONFIG_HID_U2FZERO) += hid-u2fzero.o
obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
obj-$(CONFIG_HID_UDRAW_PS3) += hid-udraw-ps3.o
obj-$(CONFIG_HID_LED) += hid-led.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 24f846d67478..36f898eab36f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -321,6 +321,7 @@
#define USB_DEVICE_ID_CYGNAL_RADIO_SI470X 0x818a
#define USB_DEVICE_ID_FOCALTECH_FTXXXX_MULTITOUCH 0x81b9
#define USB_DEVICE_ID_CYGNAL_CP2112 0xea90
+#define USB_DEVICE_ID_U2F_ZERO 0x8acf
#define USB_DEVICE_ID_CYGNAL_RADIO_SI4713 0x8244
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
new file mode 100644
index 000000000000..d11a5cb56a0d
--- /dev/null
+++ b/drivers/hid/hid-u2fzero.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * U2F Zero LED and RNG driver
+ *
+ * Copyright 2018 Andrej Shadura <andrew@shadura.me>
+ * Loosely based on drivers/hid/hid-led.c
+ * and drivers/usb/misc/chaoskey.c
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2.
+ */
+
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/hw_random.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/usb.h>
+
+#include "usbhid/usbhid.h"
+#include "hid-ids.h"
+
+#define DRIVER_SHORT "u2fzero"
+
+#define HID_REPORT_SIZE 64
+
+/* We only use broadcast (CID-less) messages */
+#define CID_BROADCAST 0xffffffff
+
+struct u2f_hid_msg {
+ u32 cid;
+ union {
+ struct {
+ u8 cmd;
+ u8 bcnth;
+ u8 bcntl;
+ u8 data[HID_REPORT_SIZE - 7];
+ } init;
+ struct {
+ u8 seq;
+ u8 data[HID_REPORT_SIZE - 5];
+ } cont;
+ };
+} __packed;
+
+struct u2f_hid_report {
+ u8 report_type;
+ struct u2f_hid_msg msg;
+} __packed;
+
+#define U2F_HID_MSG_LEN(f) (size_t)(((f).init.bcnth << 8) + (f).init.bcntl)
+
+/* Custom extensions to the U2FHID protocol */
+#define U2F_CUSTOM_GET_RNG 0x21
+#define U2F_CUSTOM_WINK 0x24
+
+struct u2fzero_device {
+ struct hid_device *hdev;
+ struct urb *urb; /* URB for the RNG data */
+ struct led_classdev ldev; /* Embedded struct for led */
+ struct hwrng hwrng; /* Embedded struct for hwrng */
+ char *led_name;
+ char *rng_name;
+ u8 *buf_out;
+ u8 *buf_in;
+ struct mutex lock;
+ bool present;
+};
+
+static int u2fzero_send(struct u2fzero_device *dev, struct u2f_hid_report *req)
+{
+ int ret;
+
+ mutex_lock(&dev->lock);
+
+ memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
+
+ ret = hid_hw_output_report(dev->hdev, dev->buf_out,
+ sizeof(struct u2f_hid_msg));
+
+ mutex_unlock(&dev->lock);
+
+ if (ret < 0)
+ return ret;
+
+ return ret == sizeof(struct u2f_hid_msg) ? 0 : -EMSGSIZE;
+}
+
+struct u2fzero_transfer_context {
+ struct completion done;
+ int status;
+};
+
+static void u2fzero_read_callback(struct urb *urb)
+{
+ struct u2fzero_transfer_context *ctx = urb->context;
+
+ ctx->status = urb->status;
+ complete(&ctx->done);
+}
+
+static int u2fzero_recv(struct u2fzero_device *dev,
+ struct u2f_hid_report *req,
+ struct u2f_hid_msg *resp)
+{
+ int ret;
+ struct hid_device *hdev = dev->hdev;
+ struct u2fzero_transfer_context ctx;
+
+ mutex_lock(&dev->lock);
+
+ memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
+
+ dev->urb->context = &ctx;
+ init_completion(&ctx.done);
+
+ ret = usb_submit_urb(dev->urb, GFP_NOIO);
+ if (unlikely(ret)) {
+ hid_err(hdev, "usb_submit_urb failed: %d", ret);
+ goto err;
+ }
+
+ ret = hid_hw_output_report(dev->hdev, dev->buf_out,
+ sizeof(struct u2f_hid_msg));
+
+ if (ret < 0) {
+ hid_err(hdev, "hid_hw_output_report failed: %d", ret);
+ goto err;
+ }
+
+ ret = (wait_for_completion_timeout(
+ &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
+ if (ret < 0) {
+ usb_kill_urb(dev->urb);
+ hid_err(hdev, "urb submission timed out");
+ } else {
+ ret = dev->urb->actual_length;
+ memcpy(resp, dev->buf_in, ret);
+ }
+
+err:
+ mutex_unlock(&dev->lock);
+
+ return ret;
+}
+
+static int u2fzero_blink(struct led_classdev *ldev)
+{
+ struct u2fzero_device *dev = container_of(ldev,
+ struct u2fzero_device, ldev);
+ struct u2f_hid_report req = {
+ .report_type = 0,
+ .msg.cid = CID_BROADCAST,
+ .msg.init = {
+ .cmd = U2F_CUSTOM_WINK,
+ .bcnth = 0,
+ .bcntl = 0,
+ .data = {0},
+ }
+ };
+ return u2fzero_send(dev, &req);
+}
+
+static int u2fzero_brightness_set(struct led_classdev *ldev,
+ enum led_brightness brightness)
+{
+ ldev->brightness = LED_OFF;
+ if (brightness)
+ return u2fzero_blink(ldev);
+ else
+ return 0;
+}
+
+static int u2fzero_rng_read(struct hwrng *rng, void *data,
+ size_t max, bool wait)
+{
+ struct u2fzero_device *dev = container_of(rng,
+ struct u2fzero_device, hwrng);
+ struct u2f_hid_report req = {
+ .report_type = 0,
+ .msg.cid = CID_BROADCAST,
+ .msg.init = {
+ .cmd = U2F_CUSTOM_GET_RNG,
+ .bcnth = 0,
+ .bcntl = 0,
+ .data = {0},
+ }
+ };
+ struct u2f_hid_msg resp;
+ int ret;
+ size_t actual_length;
+
+ if (!dev->present) {
+ hid_dbg(dev->hdev, "device not present");
+ return 0;
+ }
+
+ ret = u2fzero_recv(dev, &req, &resp);
+ if (ret < 0)
+ return 0;
+
+ /* only take the minimum amount of data it is safe to take */
+ actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
+ init.data), U2F_HID_MSG_LEN(resp), max);
+
+ memcpy(data, resp.init.data, actual_length);
+
+ return actual_length;
+}
+
+static int u2fzero_init_led(struct u2fzero_device *dev,
+ unsigned int minor)
+{
+ dev->led_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
+ "%s%u", DRIVER_SHORT, minor);
+ if (dev->led_name == NULL)
+ return -ENOMEM;
+
+ dev->ldev.name = dev->led_name;
+ dev->ldev.max_brightness = LED_ON;
+ dev->ldev.flags = LED_HW_PLUGGABLE;
+ dev->ldev.brightness_set_blocking = u2fzero_brightness_set;
+
+ return devm_led_classdev_register(&dev->hdev->dev, &dev->ldev);
+}
+
+static int u2fzero_init_hwrng(struct u2fzero_device *dev,
+ unsigned int minor)
+{
+ dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
+ "%s-rng%u", DRIVER_SHORT, minor);
+ if (dev->rng_name == NULL)
+ return -ENOMEM;
+
+ dev->hwrng.name = dev->rng_name;
+ dev->hwrng.read = u2fzero_rng_read;
+ dev->hwrng.quality = 1;
+
+ return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng);
+}
+
+static int u2fzero_fill_in_urb(struct u2fzero_device *dev)
+{
+ struct hid_device *hdev = dev->hdev;
+ struct usb_device *udev;
+ struct usbhid_device *usbhid = hdev->driver_data;
+ unsigned int pipe_in;
+ struct usb_host_endpoint *ep;
+
+ if (dev->hdev->bus != BUS_USB)
+ return -EINVAL;
+
+ udev = hid_to_usb_dev(hdev);
+
+ if (!usbhid->urbout || !usbhid->urbin)
+ return -ENODEV;
+
+ ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe);
+ if (!ep)
+ return -ENODEV;
+
+ dev->urb = usb_alloc_urb(0, GFP_KERNEL);
+ if (!dev->urb)
+ return -ENOMEM;
+
+ pipe_in = (usbhid->urbin->pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
+
+ usb_fill_int_urb(dev->urb,
+ udev,
+ pipe_in,
+ dev->buf_in,
+ HID_REPORT_SIZE,
+ u2fzero_read_callback,
+ NULL,
+ ep->desc.bInterval);
+
+ return 0;
+}
+
+static int u2fzero_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct u2fzero_device *dev;
+ unsigned int minor;
+ int ret;
+
+ dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (dev == NULL)
+ return -ENOMEM;
+
+ dev->buf_out = devm_kmalloc(&hdev->dev,
+ sizeof(struct u2f_hid_report), GFP_KERNEL);
+ if (dev->buf_out == NULL)
+ return -ENOMEM;
+
+ dev->buf_in = devm_kmalloc(&hdev->dev,
+ sizeof(struct u2f_hid_msg), GFP_KERNEL);
+ if (dev->buf_in == NULL)
+ return -ENOMEM;
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ dev->hdev = hdev;
+ hid_set_drvdata(hdev, dev);
+ mutex_init(&dev->lock);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
+ return ret;
+
+ u2fzero_fill_in_urb(dev);
+
+ dev->present = true;
+
+ minor = ((struct hidraw *) hdev->hidraw)->minor;
+
+ ret = u2fzero_init_led(dev, minor);
+ if (ret) {
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ hid_info(hdev, "U2F Zero LED initialised\n");
+
+ ret = u2fzero_init_hwrng(dev, minor);
+ if (ret) {
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ hid_info(hdev, "U2F Zero RNG initialised\n");
+
+ return 0;
+}
+
+static void u2fzero_remove(struct hid_device *hdev)
+{
+ struct u2fzero_device *dev = hid_get_drvdata(hdev);
+
+ mutex_lock(&dev->lock);
+ dev->present = false;
+ mutex_unlock(&dev->lock);
+
+ hid_hw_stop(hdev);
+ usb_poison_urb(dev->urb);
+ usb_free_urb(dev->urb);
+}
+
+static const struct hid_device_id u2fzero_table[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL,
+ USB_DEVICE_ID_U2F_ZERO) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, u2fzero_table);
+
+static struct hid_driver u2fzero_driver = {
+ .name = "hid-" DRIVER_SHORT,
+ .probe = u2fzero_probe,
+ .remove = u2fzero_remove,
+ .id_table = u2fzero_table,
+};
+
+module_hid_driver(u2fzero_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Andrej Shadura <andrew@shadura.me>");
+MODULE_DESCRIPTION("U2F Zero LED and RNG driver");
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-10 12:02 ` Jiri Kosina
2019-04-10 12:02 ` [PATCH v3] " Jiri Kosina
2019-04-11 9:58 ` [v3] " Benjamin Tissoires
0 siblings, 2 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-10 12:02 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-input, linux-usb, Andrzej Pietrasiewicz, kernel,
Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Mon, 1 Apr 2019, Andrej Shadura wrote:
> U2F Zero supports custom commands for blinking the LED and getting data
> from the internal hardware RNG. Expose the blinking function as a LED
> device, and the internal hardware RNG as an HWRNG so that it can be used
> to feed the enthropy pool.
So I still am not really happy about this being wired up into generic HID
although it's really a USB driver, but we've discussed that already, and I
don't see any option that'd work substantially better in this case,
especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-10 12:02 ` Jiri Kosina
@ 2019-04-10 12:02 ` Jiri Kosina
2019-04-11 9:58 ` [v3] " Benjamin Tissoires
1 sibling, 0 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-10 12:02 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-input, linux-usb, Andrzej Pietrasiewicz, kernel,
Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Mon, 1 Apr 2019, Andrej Shadura wrote:
> U2F Zero supports custom commands for blinking the LED and getting data
> from the internal hardware RNG. Expose the blinking function as a LED
> device, and the internal hardware RNG as an HWRNG so that it can be used
> to feed the enthropy pool.
So I still am not really happy about this being wired up into generic HID
although it's really a USB driver, but we've discussed that already, and I
don't see any option that'd work substantially better in this case,
especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-11 9:58 ` Benjamin Tissoires
2019-04-11 9:58 ` [PATCH v3] " Benjamin Tissoires
2019-04-11 10:35 ` [v3] " Jiri Kosina
0 siblings, 2 replies; 15+ messages in thread
From: Benjamin Tissoires @ 2019-04-11 9:58 UTC (permalink / raw)
To: Jiri Kosina
Cc: Andrej Shadura, linux-input, linux-usb, Andrzej Pietrasiewicz,
kernel, Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Wed, Apr 10, 2019 at 3:14 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Mon, 1 Apr 2019, Andrej Shadura wrote:
>
> > U2F Zero supports custom commands for blinking the LED and getting data
> > from the internal hardware RNG. Expose the blinking function as a LED
> > device, and the internal hardware RNG as an HWRNG so that it can be used
> > to feed the enthropy pool.
>
> So I still am not really happy about this being wired up into generic HID
> although it's really a USB driver, but we've discussed that already, and I
> don't see any option that'd work substantially better in this case,
> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> thanks!
>
Hi wish I were CC-ed on these threads.
My CI script now fails because hid-u2fzero.ko needs
devm_hwrng_register and the Kconfig doesn't force pulling the right
dependency.
Also, you probably want to add in .probe() a check for the actual
transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
you can not use the usbhid functions without crashing the kernel.
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-11 9:58 ` [v3] " Benjamin Tissoires
@ 2019-04-11 9:58 ` Benjamin Tissoires
2019-04-11 10:35 ` [v3] " Jiri Kosina
1 sibling, 0 replies; 15+ messages in thread
From: Benjamin Tissoires @ 2019-04-11 9:58 UTC (permalink / raw)
To: Jiri Kosina
Cc: Andrej Shadura, linux-input, linux-usb, Andrzej Pietrasiewicz,
kernel, Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Wed, Apr 10, 2019 at 3:14 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Mon, 1 Apr 2019, Andrej Shadura wrote:
>
> > U2F Zero supports custom commands for blinking the LED and getting data
> > from the internal hardware RNG. Expose the blinking function as a LED
> > device, and the internal hardware RNG as an HWRNG so that it can be used
> > to feed the enthropy pool.
>
> So I still am not really happy about this being wired up into generic HID
> although it's really a USB driver, but we've discussed that already, and I
> don't see any option that'd work substantially better in this case,
> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> thanks!
>
Hi wish I were CC-ed on these threads.
My CI script now fails because hid-u2fzero.ko needs
devm_hwrng_register and the Kconfig doesn't force pulling the right
dependency.
Also, you probably want to add in .probe() a check for the actual
transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
you can not use the usbhid functions without crashing the kernel.
Cheers,
Benjamin
^ permalink raw reply [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-11 10:35 ` Jiri Kosina
2019-04-11 10:35 ` [PATCH v3] " Jiri Kosina
2019-04-11 11:52 ` [v3] " Andrej Shadura
0 siblings, 2 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-11 10:35 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Andrej Shadura, linux-input, linux-usb, Andrzej Pietrasiewicz,
kernel, Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Thu, 11 Apr 2019, Benjamin Tissoires wrote:
> > So I still am not really happy about this being wired up into generic HID
> > although it's really a USB driver, but we've discussed that already, and I
> > don't see any option that'd work substantially better in this case,
> > especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> > thanks!
> >
>
> Hi wish I were CC-ed on these threads.
Gah, for some reason I was convinced you were.
> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
> and the Kconfig doesn't force pulling the right dependency.
>
> Also, you probably want to add in .probe() a check for the actual
> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
> you can not use the usbhid functions without crashing the kernel.
Benjamin, I love your CI :)
Andrej, could you please send fixups on top of the applied patch?
Thanks,
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-11 10:35 ` [v3] " Jiri Kosina
@ 2019-04-11 10:35 ` Jiri Kosina
2019-04-11 11:52 ` [v3] " Andrej Shadura
1 sibling, 0 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-11 10:35 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Andrej Shadura, linux-input, linux-usb, Andrzej Pietrasiewicz,
kernel, Theodore Ts'o, Vasily Khoruzhick, Nick Kossifidis,
linux-crypto
On Thu, 11 Apr 2019, Benjamin Tissoires wrote:
> > So I still am not really happy about this being wired up into generic HID
> > although it's really a USB driver, but we've discussed that already, and I
> > don't see any option that'd work substantially better in this case,
> > especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> > thanks!
> >
>
> Hi wish I were CC-ed on these threads.
Gah, for some reason I was convinced you were.
> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
> and the Kconfig doesn't force pulling the right dependency.
>
> Also, you probably want to add in .probe() a check for the actual
> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
> you can not use the usbhid functions without crashing the kernel.
Benjamin, I love your CI :)
Andrej, could you please send fixups on top of the applied patch?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-11 11:52 ` Andrej Shadura
2019-04-11 11:52 ` [PATCH v3] " Andrej Shadura
2019-04-17 14:43 ` [v3] " Jiri Kosina
0 siblings, 2 replies; 15+ messages in thread
From: Andrej Shadura @ 2019-04-11 11:52 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-usb, Andrzej Pietrasiewicz, kernel, linux-crypto
On 11/04/2019 12:35, Jiri Kosina wrote:
> On Thu, 11 Apr 2019, Benjamin Tissoires wrote:
>
>>> So I still am not really happy about this being wired up into generic HID
>>> although it's really a USB driver, but we've discussed that already, and I
>>> don't see any option that'd work substantially better in this case,
>>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
>>> thanks!
>>>
>>
>> Hi wish I were CC-ed on these threads.
>
> Gah, for some reason I was convinced you were.
>
>> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
>> and the Kconfig doesn't force pulling the right dependency.
>> Also, you probably want to add in .probe() a check for the actual
>> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
>> you can not use the usbhid functions without crashing the kernel.
Thanks, that’s very useful.
> Benjamin, I love your CI :)
>
> Andrej, could you please send fixups on top of the applied patch?
Sure, will do.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-11 11:52 ` [v3] " Andrej Shadura
@ 2019-04-11 11:52 ` Andrej Shadura
2019-04-17 14:43 ` [v3] " Jiri Kosina
1 sibling, 0 replies; 15+ messages in thread
From: Andrej Shadura @ 2019-04-11 11:52 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-usb, Andrzej Pietrasiewicz, kernel, linux-crypto
On 11/04/2019 12:35, Jiri Kosina wrote:
> On Thu, 11 Apr 2019, Benjamin Tissoires wrote:
>
>>> So I still am not really happy about this being wired up into generic HID
>>> although it's really a USB driver, but we've discussed that already, and I
>>> don't see any option that'd work substantially better in this case,
>>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
>>> thanks!
>>>
>>
>> Hi wish I were CC-ed on these threads.
>
> Gah, for some reason I was convinced you were.
>
>> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
>> and the Kconfig doesn't force pulling the right dependency.
>> Also, you probably want to add in .probe() a check for the actual
>> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
>> you can not use the usbhid functions without crashing the kernel.
Thanks, that’s very useful.
> Benjamin, I love your CI :)
>
> Andrej, could you please send fixups on top of the applied patch?
Sure, will do.
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-17 14:43 ` Jiri Kosina
2019-04-17 14:43 ` [PATCH v3] " Jiri Kosina
2019-04-17 14:47 ` [v3] " Andrej Shadura
0 siblings, 2 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-17 14:43 UTC (permalink / raw)
To: Andrej Shadura
Cc: Benjamin Tissoires, linux-input, linux-usb,
Andrzej Pietrasiewicz, kernel, linux-crypto
On Thu, 11 Apr 2019, Andrej Shadura wrote:
> >>> So I still am not really happy about this being wired up into generic HID
> >>> although it's really a USB driver, but we've discussed that already, and I
> >>> don't see any option that'd work substantially better in this case,
> >>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> >>> thanks!
> >>>
> >>
> >> Hi wish I were CC-ed on these threads.
> >
> > Gah, for some reason I was convinced you were.
> >
> >> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
> >> and the Kconfig doesn't force pulling the right dependency.
>
> >> Also, you probably want to add in .probe() a check for the actual
> >> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
> >> you can not use the usbhid functions without crashing the kernel.
>
> Thanks, that’s very useful.
>
> > Benjamin, I love your CI :)
> >
> > Andrej, could you please send fixups on top of the applied patch?
>
> Sure, will do.
I didn't receive any followup; the Kconfig dependency has already been
fixed by Mao Wenan, and I've just queued the patch below on top of that as
well.
From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] HID: u2fzero: fail probe if not using USB transport
u2fzero driver is USB-only. Therefore we have to give up in ->probe()
callback in case we're called with non-USB transport driver bound,
otherwise the kernel will crash trying to use USBHID API on a non-USB
transport.
Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
drivers/hid/hid-u2fzero.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index d11a5cb56a0d..95e0807878c7 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -286,6 +286,9 @@ static int u2fzero_probe(struct hid_device *hdev,
unsigned int minor;
int ret;
+ if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
+ return -EINVAL;
+
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
if (dev == NULL)
return -ENOMEM;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-17 14:43 ` [v3] " Jiri Kosina
@ 2019-04-17 14:43 ` Jiri Kosina
2019-04-17 14:47 ` [v3] " Andrej Shadura
1 sibling, 0 replies; 15+ messages in thread
From: Jiri Kosina @ 2019-04-17 14:43 UTC (permalink / raw)
To: Andrej Shadura
Cc: Benjamin Tissoires, linux-input, linux-usb,
Andrzej Pietrasiewicz, kernel, linux-crypto
On Thu, 11 Apr 2019, Andrej Shadura wrote:
> >>> So I still am not really happy about this being wired up into generic HID
> >>> although it's really a USB driver, but we've discussed that already, and I
> >>> don't see any option that'd work substantially better in this case,
> >>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
> >>> thanks!
> >>>
> >>
> >> Hi wish I were CC-ed on these threads.
> >
> > Gah, for some reason I was convinced you were.
> >
> >> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
> >> and the Kconfig doesn't force pulling the right dependency.
>
> >> Also, you probably want to add in .probe() a check for the actual
> >> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
> >> you can not use the usbhid functions without crashing the kernel.
>
> Thanks, that’s very useful.
>
> > Benjamin, I love your CI :)
> >
> > Andrej, could you please send fixups on top of the applied patch?
>
> Sure, will do.
I didn't receive any followup; the Kconfig dependency has already been
fixed by Mao Wenan, and I've just queued the patch below on top of that as
well.
From: Jiri Kosina <jkosina@suse.cz>
Subject: [PATCH] HID: u2fzero: fail probe if not using USB transport
u2fzero driver is USB-only. Therefore we have to give up in ->probe()
callback in case we're called with non-USB transport driver bound,
otherwise the kernel will crash trying to use USBHID API on a non-USB
transport.
Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
drivers/hid/hid-u2fzero.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index d11a5cb56a0d..95e0807878c7 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -286,6 +286,9 @@ static int u2fzero_probe(struct hid_device *hdev,
unsigned int minor;
int ret;
+ if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
+ return -EINVAL;
+
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
if (dev == NULL)
return -ENOMEM;
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [v3] HID: add driver for U2F Zero built-in LED and RNG
@ 2019-04-17 14:47 ` Andrej Shadura
2019-04-17 14:47 ` [PATCH v3] " Andrej Shadura
0 siblings, 1 reply; 15+ messages in thread
From: Andrej Shadura @ 2019-04-17 14:47 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-usb,
Andrzej Pietrasiewicz, kernel, linux-crypto
On 17/04/2019 16:43, Jiri Kosina wrote:
> On Thu, 11 Apr 2019, Andrej Shadura wrote:
>
>>>>> So I still am not really happy about this being wired up into generic HID
>>>>> although it's really a USB driver, but we've discussed that already, and I
>>>>> don't see any option that'd work substantially better in this case,
>>>>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
>>>>> thanks!
>>>>>
>>>>
>>>> Hi wish I were CC-ed on these threads.
>>>
>>> Gah, for some reason I was convinced you were.
>>>
>>>> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
>>>> and the Kconfig doesn't force pulling the right dependency.
>>
>>>> Also, you probably want to add in .probe() a check for the actual
>>>> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
>>>> you can not use the usbhid functions without crashing the kernel.
>>
>> Thanks, that’s very useful.
>>
>>> Benjamin, I love your CI :)
>>>
>>> Andrej, could you please send fixups on top of the applied patch?
>>
>> Sure, will do.
>
> I didn't receive any followup; the Kconfig dependency has already been
> fixed by Mao Wenan, and I've just queued the patch below on top of that as
> well.
Sorry, I’ve had too much on my plate in the recent days, so I’ve
unfortunately been pushing this task off a bit. Thanks to both of you,
Mao and Jiří, both things should have really been done by me. I’ll try
to get those things right from the beginning the next time.
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: u2fzero: fail probe if not using USB transport
>
> u2fzero driver is USB-only. Therefore we have to give up in ->probe()
> callback in case we're called with non-USB transport driver bound,
> otherwise the kernel will crash trying to use USBHID API on a non-USB
> transport.
>
> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
> Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/hid/hid-u2fzero.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> index d11a5cb56a0d..95e0807878c7 100644
> --- a/drivers/hid/hid-u2fzero.c
> +++ b/drivers/hid/hid-u2fzero.c
> @@ -286,6 +286,9 @@ static int u2fzero_probe(struct hid_device *hdev,
> unsigned int minor;
> int ret;
>
> + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
> + return -EINVAL;
> +
> dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> if (dev == NULL)
> return -ENOMEM;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-17 14:47 ` [v3] " Andrej Shadura
@ 2019-04-17 14:47 ` Andrej Shadura
0 siblings, 0 replies; 15+ messages in thread
From: Andrej Shadura @ 2019-04-17 14:47 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, linux-input, linux-usb,
Andrzej Pietrasiewicz, kernel, linux-crypto
On 17/04/2019 16:43, Jiri Kosina wrote:
> On Thu, 11 Apr 2019, Andrej Shadura wrote:
>
>>>>> So I still am not really happy about this being wired up into generic HID
>>>>> although it's really a USB driver, but we've discussed that already, and I
>>>>> don't see any option that'd work substantially better in this case,
>>>>> especially from the UX point of view. Oh well. Applied to for-5.2/u2fzero,
>>>>> thanks!
>>>>>
>>>>
>>>> Hi wish I were CC-ed on these threads.
>>>
>>> Gah, for some reason I was convinced you were.
>>>
>>>> My CI script now fails because hid-u2fzero.ko needs devm_hwrng_register
>>>> and the Kconfig doesn't force pulling the right dependency.
>>
>>>> Also, you probably want to add in .probe() a check for the actual
>>>> transport driver (`hid_is_using_ll_driver(hdev, &usb_hid_driver)`) or
>>>> you can not use the usbhid functions without crashing the kernel.
>>
>> Thanks, that’s very useful.
>>
>>> Benjamin, I love your CI :)
>>>
>>> Andrej, could you please send fixups on top of the applied patch?
>>
>> Sure, will do.
>
> I didn't receive any followup; the Kconfig dependency has already been
> fixed by Mao Wenan, and I've just queued the patch below on top of that as
> well.
Sorry, I’ve had too much on my plate in the recent days, so I’ve
unfortunately been pushing this task off a bit. Thanks to both of you,
Mao and Jiří, both things should have really been done by me. I’ll try
to get those things right from the beginning the next time.
> From: Jiri Kosina <jkosina@suse.cz>
> Subject: [PATCH] HID: u2fzero: fail probe if not using USB transport
>
> u2fzero driver is USB-only. Therefore we have to give up in ->probe()
> callback in case we're called with non-USB transport driver bound,
> otherwise the kernel will crash trying to use USBHID API on a non-USB
> transport.
>
> Fixes: 42337b9d4d958("HID: add driver for U2F Zero built-in LED and RNG")
> Reported-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/hid/hid-u2fzero.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
> index d11a5cb56a0d..95e0807878c7 100644
> --- a/drivers/hid/hid-u2fzero.c
> +++ b/drivers/hid/hid-u2fzero.c
> @@ -286,6 +286,9 @@ static int u2fzero_probe(struct hid_device *hdev,
> unsigned int minor;
> int ret;
>
> + if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
> + return -EINVAL;
> +
> dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> if (dev == NULL)
> return -ENOMEM;
>
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-04-01 12:42 [v3] HID: add driver for U2F Zero built-in LED and RNG Andrej Shadura
2019-04-10 12:02 ` Jiri Kosina
@ 2019-10-03 7:19 ` Andrej Shadura
2019-10-03 15:25 ` Alan Stern
1 sibling, 1 reply; 15+ messages in thread
From: Andrej Shadura @ 2019-10-03 7:19 UTC (permalink / raw)
To: linux-usb
Cc: Jiří Kosina, linux-input, Andrzej Pietrasiewicz,
kernel, Ezequiel Garcia, Vasily Khoruzhick
On 01/04/2019 14:42, Andrej Shadura wrote:
> U2F Zero supports custom commands for blinking the LED and getting data
> from the internal hardware RNG. Expose the blinking function as a LED
> device, and the internal hardware RNG as an HWRNG so that it can be used
> to feed the enthropy pool.
>
> Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
I’ve been testing this with a different modification of U2F Zero,
Nitrokey FIDO U2F, and on that device only I’m getting a kernel warning
(see below).
> +static int u2fzero_recv(struct u2fzero_device *dev,
> + struct u2f_hid_report *req,
> + struct u2f_hid_msg *resp)
> +{
> + int ret;
> + struct hid_device *hdev = dev->hdev;
> + struct u2fzero_transfer_context ctx;
> +
> + mutex_lock(&dev->lock);
> +
> + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> +
> + dev->urb->context = &ctx;
> + init_completion(&ctx.done);
> +
> + ret = usb_submit_urb(dev->urb, GFP_NOIO);
Here, usb_submit_urb() returns EBUSY in the first attempt to read random
numbers from the device:
URB ffff917256d5d540 submitted while active
WARNING: CPU: 3 PID: 31 at drivers/usb/core/urb.c:363
usb_submit_urb+0x4c2/0x5b0
<...>
Call Trace:
u2fzero_rng_read+0x16e/0x340 [hid_u2fzero]
? ttwu_do_activate+0x67/0x90
add_early_randomness+0x53/0xc0
hwrng_register+0x175/0x180
devm_hwrng_register+0x41/0x7e
u2fzero_probe+0x2dd/0x350 [hid_u2fzero]
hid_device_probe+0x119/0x180 [hid]
really_probe+0xfe/0x3b0
driver_probe_device+0xba/0x100
__device_attach_driver+0x97/0x100
I don’t understand why since 1) it’s likely to be the first transmission
of this URB, 2) there’s a mutex locked just before it. I received a
comment from a colleague mentioning I’m probably not using the mutex
correctly, but I don’t understand why.
I’m trying to figure this out, so I’d welcome any help with it.
Thanks in advance.
> + if (unlikely(ret)) {
> + hid_err(hdev, "usb_submit_urb failed: %d", ret);
> + goto err;
> + }
> +
> + ret = hid_hw_output_report(dev->hdev, dev->buf_out,
> + sizeof(struct u2f_hid_msg));
> +
> + if (ret < 0) {
> + hid_err(hdev, "hid_hw_output_report failed: %d", ret);
> + goto err;
> + }
> +
> + ret = (wait_for_completion_timeout(
> + &ctx.done, msecs_to_jiffies(USB_CTRL_SET_TIMEOUT)));
> + if (ret < 0) {
> + usb_kill_urb(dev->urb);
> + hid_err(hdev, "urb submission timed out");
> + } else {
> + ret = dev->urb->actual_length;
> + memcpy(resp, dev->buf_in, ret);
> + }
> +
> +err:
> + mutex_unlock(&dev->lock);
> +
> + return ret;
> +}
<...>
> +static int u2fzero_rng_read(struct hwrng *rng, void *data,
> + size_t max, bool wait)
> +{
> + struct u2fzero_device *dev = container_of(rng,
> + struct u2fzero_device, hwrng);
> + struct u2f_hid_report req = {
> + .report_type = 0,
> + .msg.cid = CID_BROADCAST,
> + .msg.init = {
> + .cmd = U2F_CUSTOM_GET_RNG,
> + .bcnth = 0,
> + .bcntl = 0,
> + .data = {0},
> + }
> + };
> + struct u2f_hid_msg resp;
> + int ret;
> + size_t actual_length;
> +
> + if (!dev->present) {
> + hid_dbg(dev->hdev, "device not present");
> + return 0;
> + }
> +
> + ret = u2fzero_recv(dev, &req, &resp);
> + if (ret < 0)
> + return 0;
> +
> + /* only take the minimum amount of data it is safe to take */
> + actual_length = min3((size_t)ret - offsetof(struct u2f_hid_msg,
> + init.data), U2F_HID_MSG_LEN(resp), max);
> +
> + memcpy(data, resp.init.data, actual_length);
> +
> + return actual_length;
> +}
<...>
> +static int u2fzero_init_hwrng(struct u2fzero_device *dev,
> + unsigned int minor)
> +{
> + dev->rng_name = devm_kasprintf(&dev->hdev->dev, GFP_KERNEL,
> + "%s-rng%u", DRIVER_SHORT, minor);
> + if (dev->rng_name == NULL)
> + return -ENOMEM;
> +
> + dev->hwrng.name = dev->rng_name;
> + dev->hwrng.read = u2fzero_rng_read;
> + dev->hwrng.quality = 1;
> +
> + return devm_hwrng_register(&dev->hdev->dev, &dev->hwrng);
> +}
> +
> +static int u2fzero_fill_in_urb(struct u2fzero_device *dev)
> +{
> + struct hid_device *hdev = dev->hdev;
> + struct usb_device *udev;
> + struct usbhid_device *usbhid = hdev->driver_data;
> + unsigned int pipe_in;
> + struct usb_host_endpoint *ep;
> +
> + if (dev->hdev->bus != BUS_USB)
> + return -EINVAL;
> +
> + udev = hid_to_usb_dev(hdev);
> +
> + if (!usbhid->urbout || !usbhid->urbin)
> + return -ENODEV;
> +
> + ep = usb_pipe_endpoint(udev, usbhid->urbin->pipe);
> + if (!ep)
> + return -ENODEV;
> +
> + dev->urb = usb_alloc_urb(0, GFP_KERNEL);
> + if (!dev->urb)
> + return -ENOMEM;
> +
> + pipe_in = (usbhid->urbin->pipe & ~(3 << 30)) | (PIPE_INTERRUPT << 30);
> +
> + usb_fill_int_urb(dev->urb,
> + udev,
> + pipe_in,
> + dev->buf_in,
> + HID_REPORT_SIZE,
> + u2fzero_read_callback,
> + NULL,
> + ep->desc.bInterval);
> +
> + return 0;
> +}
> +
> +static int u2fzero_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct u2fzero_device *dev;
> + unsigned int minor;
> + int ret;
> +
> + dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
> + if (dev == NULL)
> + return -ENOMEM;
> +
> + dev->buf_out = devm_kmalloc(&hdev->dev,
> + sizeof(struct u2f_hid_report), GFP_KERNEL);
> + if (dev->buf_out == NULL)
> + return -ENOMEM;
> +
> + dev->buf_in = devm_kmalloc(&hdev->dev,
> + sizeof(struct u2f_hid_msg), GFP_KERNEL);
> + if (dev->buf_in == NULL)
> + return -ENOMEM;
> +
> + ret = hid_parse(hdev);
> + if (ret)
> + return ret;
> +
> + dev->hdev = hdev;
> + hid_set_drvdata(hdev, dev);
> + mutex_init(&dev->lock);
> +
> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
> +
> + u2fzero_fill_in_urb(dev);
> +
> + dev->present = true;
> +
> + minor = ((struct hidraw *) hdev->hidraw)->minor;
> +
> + ret = u2fzero_init_led(dev, minor);
> + if (ret) {
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + hid_info(hdev, "U2F Zero LED initialised\n");
> +
> + ret = u2fzero_init_hwrng(dev, minor);
> + if (ret) {
> + hid_hw_stop(hdev);
> + return ret;
> + }
> +
> + hid_info(hdev, "U2F Zero RNG initialised\n");
> +
> + return 0;
> +}
--
Cheers,
Andrej
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] HID: add driver for U2F Zero built-in LED and RNG
2019-10-03 7:19 ` Andrej Shadura
@ 2019-10-03 15:25 ` Alan Stern
0 siblings, 0 replies; 15+ messages in thread
From: Alan Stern @ 2019-10-03 15:25 UTC (permalink / raw)
To: Andrej Shadura
Cc: linux-usb, Jiří Kosina, linux-input,
Andrzej Pietrasiewicz, kernel, Ezequiel Garcia,
Vasily Khoruzhick
On Thu, 3 Oct 2019, Andrej Shadura wrote:
> On 01/04/2019 14:42, Andrej Shadura wrote:
> > U2F Zero supports custom commands for blinking the LED and getting data
> > from the internal hardware RNG. Expose the blinking function as a LED
> > device, and the internal hardware RNG as an HWRNG so that it can be used
> > to feed the enthropy pool.
> >
> > Signed-off-by: Andrej Shadura <andrew.shadura@collabora.co.uk>
>
> I’ve been testing this with a different modification of U2F Zero,
> Nitrokey FIDO U2F, and on that device only I’m getting a kernel warning
> (see below).
>
> > +static int u2fzero_recv(struct u2fzero_device *dev,
> > + struct u2f_hid_report *req,
> > + struct u2f_hid_msg *resp)
> > +{
> > + int ret;
> > + struct hid_device *hdev = dev->hdev;
> > + struct u2fzero_transfer_context ctx;
> > +
> > + mutex_lock(&dev->lock);
> > +
> > + memcpy(dev->buf_out, req, sizeof(struct u2f_hid_report));
> > +
> > + dev->urb->context = &ctx;
> > + init_completion(&ctx.done);
> > +
> > + ret = usb_submit_urb(dev->urb, GFP_NOIO);
>
> Here, usb_submit_urb() returns EBUSY in the first attempt to read random
> numbers from the device:
>
> URB ffff917256d5d540 submitted while active
> WARNING: CPU: 3 PID: 31 at drivers/usb/core/urb.c:363
> usb_submit_urb+0x4c2/0x5b0
> <...>
> Call Trace:
> u2fzero_rng_read+0x16e/0x340 [hid_u2fzero]
> ? ttwu_do_activate+0x67/0x90
> add_early_randomness+0x53/0xc0
> hwrng_register+0x175/0x180
> devm_hwrng_register+0x41/0x7e
> u2fzero_probe+0x2dd/0x350 [hid_u2fzero]
> hid_device_probe+0x119/0x180 [hid]
> really_probe+0xfe/0x3b0
> driver_probe_device+0xba/0x100
> __device_attach_driver+0x97/0x100
>
> I don’t understand why since 1) it’s likely to be the first transmission
> of this URB, 2) there’s a mutex locked just before it. I received a
> comment from a colleague mentioning I’m probably not using the mutex
> correctly, but I don’t understand why.
>
> I’m trying to figure this out, so I’d welcome any help with it.
You can try using usbmon to see exactly what URBs are actually running.
Alan Stern
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-10-03 15:25 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-01 12:42 [v3] HID: add driver for U2F Zero built-in LED and RNG Andrej Shadura
2019-04-10 12:02 ` Jiri Kosina
2019-04-10 12:02 ` [PATCH v3] " Jiri Kosina
2019-04-11 9:58 ` [v3] " Benjamin Tissoires
2019-04-11 9:58 ` [PATCH v3] " Benjamin Tissoires
2019-04-11 10:35 ` [v3] " Jiri Kosina
2019-04-11 10:35 ` [PATCH v3] " Jiri Kosina
2019-04-11 11:52 ` [v3] " Andrej Shadura
2019-04-11 11:52 ` [PATCH v3] " Andrej Shadura
2019-04-17 14:43 ` [v3] " Jiri Kosina
2019-04-17 14:43 ` [PATCH v3] " Jiri Kosina
2019-04-17 14:47 ` [v3] " Andrej Shadura
2019-04-17 14:47 ` [PATCH v3] " Andrej Shadura
2019-10-03 7:19 ` Andrej Shadura
2019-10-03 15:25 ` Alan Stern
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).