linux-kernel - Re: [PATCH 3/3] HID: usbhid: defer LED setting to a workqueue

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 20 Dec 2011 18:12:37 +0800
From:	Daniel Kurtz <djkurtz@...omium.org>
To:	Oliver Neukum <oneukum@...e.de>
Cc:	jkosina@...e.cz, bleung@...omium.org, stern@...land.harvard.edu,
	olofj@...omium.org, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 3/3] HID: usbhid: defer LED setting to a workqueue

On Thu, Dec 15, 2011 at 5:01 PM, Oliver Neukum <oneukum@...e.de> wrote:
>
> Am Donnerstag, 15. Dezember 2011, 07:43:09 schrieb Daniel Kurtz:
>
> Hi,
>
> > I'm sorry, I'm not seeing how using a workqueue to set the LEDs
> > changes anything with respect to how the driver deals with USB reset.
>
> You are unfortunately right. This means that the driver currently is already
> buggy.
>
> > With or without a workqueue, LED control urbs are being queued and/or
> > submitted asynchronously to reset.  AFAICT, the only thing that
> > changes here is exactly when __usbhid_submit_report() is called.  In
> > the original case, this happens directly in the input event handler.
> > In the workqueue case, it happens in a system worker thread some short
> > time after the input handler finishes.
>
> The current driver kills the ctrl URB in cease_io() which is not enough
> to prevent new IO.
>
> > Could you please be more specific about what this patch breaks and
> > perhaps give some guidance on how to fix it?
>
> It breaks nothing. It just continues a bug and I assumed it was not present.
> Basically the work queue must do nothing after pre_reset() and post_reset()
> ought to rerun the work in case some request came down during that time.

So, is this an Ack for this patchset?
Can we fix any existing races in later patches?

Regards,
-Daniel

>        Regards
>                Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/