| 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
| ||
|
Message-ID: <CABXOdTfdLv5iiZhyGmrmYfk15esoiJhshs_ey+9ma7hLqC4p2A@mail.gmail.com>
Date: Tue, 9 Jun 2020 22:28:00 -0700
From: Guenter Roeck <groeck@...gle.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Guenter Roeck <groeck@...omium.org>,
Nicolas Boichat <drinkcat@...omium.org>,
linux-usb@...r.kernel.org,
"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] HID: usbhid: do not sleep when opening device
On Tue, Jun 9, 2020 at 9:38 PM Dmitry Torokhov
<dmitry.torokhov@...il.com> wrote:
>
> usbhid tries to give the device 50 milliseconds to drain its queues when
> opening the device, but dies it naively by simply sleeping in open handler,
> which slows down device probing (and thus may affect overall boot time).
>
> However we do not need to sleep as we can instead mark a point of time in
> the future when we should start processing the events.
>
> Reported-by: Nicolas Boichat <drinkcat@...omium.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
Reviewed-by: Guenter Roeck <groeck@...omium.org>
> ---
>
> v2: switched from using jiffies to ktime_t to make sure we won't have
> issues with jiffies overflowing.
>
> drivers/hid/usbhid/hid-core.c | 53 +++++++++++++++++++----------------
> drivers/hid/usbhid/usbhid.h | 2 ++
> 2 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index c7bc9db5b192..72c92aab2b18 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -26,6 +26,7 @@
> #include <linux/wait.h>
> #include <linux/workqueue.h>
> #include <linux/string.h>
> +#include <linux/timekeeping.h>
>
> #include <linux/usb.h>
>
> @@ -95,6 +96,18 @@ static int hid_start_in(struct hid_device *hid)
> set_bit(HID_NO_BANDWIDTH, &usbhid->iofl);
> } else {
> clear_bit(HID_NO_BANDWIDTH, &usbhid->iofl);
> +
> + if (test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
> + /*
> + * In case events are generated while nobody was
> + * listening, some are released when the device
> + * is re-opened. Wait 50 msec for the queue to
> + * empty before allowing events to go through
> + * hid.
> + */
> + usbhid->input_start_time =
> + ktime_add_ms(ktime_get_coarse(), 50);
> + }
> }
> }
> spin_unlock_irqrestore(&usbhid->lock, flags);
> @@ -280,20 +293,23 @@ static void hid_irq_in(struct urb *urb)
> if (!test_bit(HID_OPENED, &usbhid->iofl))
> break;
> usbhid_mark_busy(usbhid);
> - if (!test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
> - hid_input_report(urb->context, HID_INPUT_REPORT,
> - urb->transfer_buffer,
> - urb->actual_length, 1);
> - /*
> - * autosuspend refused while keys are pressed
> - * because most keyboards don't wake up when
> - * a key is released
> - */
> - if (hid_check_keys_pressed(hid))
> - set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> - else
> - clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> + if (test_bit(HID_RESUME_RUNNING, &usbhid->iofl)) {
> + if (ktime_before(ktime_get_coarse(),
> + usbhid->input_start_time))
> + break;
> + clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
> }
> + hid_input_report(urb->context, HID_INPUT_REPORT,
> + urb->transfer_buffer, urb->actual_length, 1);
> + /*
> + * autosuspend refused while keys are pressed
> + * because most keyboards don't wake up when
> + * a key is released
> + */
> + if (hid_check_keys_pressed(hid))
> + set_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> + else
> + clear_bit(HID_KEYS_PRESSED, &usbhid->iofl);
> break;
> case -EPIPE: /* stall */
> usbhid_mark_busy(usbhid);
> @@ -714,17 +730,6 @@ static int usbhid_open(struct hid_device *hid)
> }
>
> usb_autopm_put_interface(usbhid->intf);
> -
> - /*
> - * In case events are generated while nobody was listening,
> - * some are released when the device is re-opened.
> - * Wait 50 msec for the queue to empty before allowing events
> - * to go through hid.
> - */
> - if (res == 0)
> - msleep(50);
> -
> - clear_bit(HID_RESUME_RUNNING, &usbhid->iofl);
> return res;
> }
>
> diff --git a/drivers/hid/usbhid/usbhid.h b/drivers/hid/usbhid/usbhid.h
> index 8620408bd7af..0f0bcf7037f8 100644
> --- a/drivers/hid/usbhid/usbhid.h
> +++ b/drivers/hid/usbhid/usbhid.h
> @@ -13,6 +13,7 @@
>
> #include <linux/types.h>
> #include <linux/slab.h>
> +#include <linux/ktime.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
> #include <linux/timer.h>
> @@ -82,6 +83,7 @@ struct usbhid_device {
>
> spinlock_t lock; /* fifo spinlock */
> unsigned long iofl; /* I/O flags (CTRL_RUNNING, OUT_RUNNING) */
> + ktime_t input_start_time; /* When to start handling input */
> struct timer_list io_retry; /* Retry timer */
> unsigned long stop_retry; /* Time to give up, in jiffies */
> unsigned int retry_delay; /* Delay length in ms */
> --
> 2.27.0.278.ge193c7cf3a9-goog
>
>
> --
> Dmitry
Powered by blists - more mailing lists