On Thu, Dec 11, 2014 at 12:49 AM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
> Hi
>
> On Wed, Dec 10, 2014 at 6:07 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>> On Tue, Dec 9, 2014 at 12:46 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>>> On Mon, Nov 3, 2014 at 12:41 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>>>> On Mon, Nov 3, 2014 at 12:21 PM, Jiri Kosina <jkosina@xxxxxxx> wrote:
>>>>> On Mon, 3 Nov 2014, David Herrmann wrote:
>>>>>
>>>>>> > Agreed, mostly. My only real concern is that this could be annoying
>>>>>> > for the userspace developers who will need to target Linux and HIDAPI
>>>>>> > separately. Admittedly the Linux support will be trivial.
>>>>>>
>>>>>> I see. I'll not stop you from using hidraw, I'd just not recommend it.
>>>>>> Especially for security stuff I dislike exposing the whole HID device
>>>>>> writable. But yeah, I guess you got my point.
>>>>>
>>>>> Alright, I am basically thinking loudly now, but how about we allow HID
>>>>> drivers that would override default hidraw interface?
>>>>>
>>>>> I am very well aware of the fact that this could be opening a can of
>>>>> worms, so we'll have to make it very restrictive. How about, let's say, we
>>>>> allow HID drivers to provide custom hidraw interface (completely
>>>>> overriding the one that HID core would normally create) only for cases
>>>>> such as:
>>>>>
>>>>> - the intent is to expose only certain parts of a combined device
>>>>> - the intent is to introduce some level of access control
>>>>>
>>>>> I.e. still no interference of kernel with data parsing allowed.
>>>>
>>>> Hmm. Would this be like a filter on hidraw actions?
>>>>
>>>> How would udev distinguish these special hidraw devices from normal
>>>> hidraw devices?
>>>>
>>>> Also, for U2F, this could be a little awkward. There's some crazy
>>>> fragmentation stuff to allow a U2F request to be split across HID
>>>> requests, and I think a kernel driver would much rather get the
>>>> original unfragmented application request.
>>>>
>>>
>>> I started writing a driver for this. I got enumeration working. I
>>> assume I should create a "u2f" device class, and then... ?
>>>
>>> Where am I supposed to get my character device from? Do I register my
>>> own chrdev major? Do I use misc? Is there some input thing I'm
>>> supposed to use?
>>
>> Another question:
>>
>> I'm hitting this in hid_input_report:
>>
>> if (down_trylock(&hid->driver_input_lock)) {
>> pr_err("HID: trylock failed\n"); // I added this
>> return -EBUSY;
>> }
>>
>> This is a problem for u2f: u2f reports are actual protocol messages,
>> and there isn't a retransmit mechanism. Losing messages randomly
>> causes the handshake to fail, and then nothing works.
>>
>> I *think* that the only way I can hit that failure is during probe (or
>> if the USB stack somehow completes two transfers at once). This still
>> makes it quite awkward to start IO from the probe routine.
>>
>> I could start a work item to take driver_input_lock, release it again,
>> and then start the handshake, but that sucks.
>
> HID reports are ordered. It's the responsibility of the
> transport-layer to not provide multiple reports in parallel. The
> reason we have this down_trylock() is to drop packages if no driver is
> loaded, yet. I wonder how you can trigger this? Does this happen
> during runtime or only driver load? Do you send GET_REPORT requests to
> the device?
It happens during driver probe. I don't set GET_REPORT to my device
-- the protocol consists solely of SET_REPORT from the driver followed
by an asynchronous (but generally reasonably quick) report back from
the device.
During probe, I set INIT, and the device responds by saying "I'm a
device. I speak this version of the protocol.
Shouldn't that code be more like:
mutex_lock(whatever);
if (driver loaded)
deliver the report;
mutex_unlock(whatever);
and then the core could hold the lock briefly as well when probing or
in hid_hw_open/hid_hw_close.
(Also, shouldn't hid_hw_open have some kind of reference counting to
avoid interference between hidraw and real drivers? Or does that
already work correctly somehow?)
--Andy
>
> Thanks
> David
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
