On Mon, Nov 3, 2014 at 5:47 PM, Andy Lutomirski <l...@amacapital.net> wrote:
> On Mon, Nov 3, 2014 at 5:32 AM, Tom Gundersen <t...@jklm.no> wrote:
>> Hi Andy,
>>
>> On Tue, Oct 28, 2014 at 11:46 PM, Andy Lutomirski <l...@amacapital.net>
>> wrote:
>>> So far, hidraw_id detects U2F tokens and sets:
>>> ID_U2F_TOKEN=1
>>> ID_SECURITY_TOKEN=1
>>>
>>> This causes the uaccess rules to apply to U2F devices.
>>> ---
>>>
>>> I've never written any udev code before. Feedback welcome.
>>>
>>> If you think this doesn't belong in udev, I can try to find it another home.
>>
>> As mentioned, I don't think we should do this in core udev code (udev
>> rule would be ok), but making it generic and exposing all the usages
>> sounds useful. I don't have a strong opinion on whether this should be
>> in udev or in the kernel, but if you end up going with udev, here are
>> some comments.
>
> Thanks. I'll wait to see what Jiri thinks before sending an updated version.
>
>>
>> Also, we don't want to add any more external helpers to the udev
>> codebase, so we should just make this a builtin (analogous to
>> udev-builtin-usb_id.c).
>>
>> Minor comments inline.
>>
>> [...]
>>
>>> diff --git a/src/udev/hidraw_id/hidraw_id.c b/src/udev/hidraw_id/hidraw_id.c
>>> new file mode 100644
>>> index 000000000000..e32f222f22f9
>>> --- /dev/null
>>> +++ b/src/udev/hidraw_id/hidraw_id.c
>>> @@ -0,0 +1,144 @@
>>> +/*
>>> + * Copyright (c) Andrew Lutomirski, 2014
>>> + *
>>> + * 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
>>
>> New udev code must be LGPL2+.
>
> OK.
>
> [snipped things I'll change if I resubmit]
>
>>
>>> + /* Long item; skip it. */
>>> + if (i + 1 >= desclen) {
>>> + log_error("bad report_descriptor");
>>> + goto out;
>>> + }
>>> + i += (desc[i+1] + 3); /* Can't overflow. */
>>> + continue;
>>> + }
>>> +
>>> + size = (sizecode < 3 ? sizecode : 4);
>>> +
>>> + if (i + 1 + size > desclen) {
>>> + log_error("bad report_descriptor");
>>> + goto out;
>>> + }
>>> +
>>> + for (j = 0; j < size; j++)
>>> + value |= (desc[i + 1 + j] << 8*j);
>>> +
>>> + if (type == 1 && tag == 0)
>>> + usage_page = value;
>>
>> Should we verify that the data has the right length (2 or 4 bytes)? I
>> guess we also need to handle 2-byte usage pages specially?
>
> I think we're not supposed to check the length in general, but I
> should re-read the part about two-byte usage pages. (Although, off
> the top of my head, I thought that the special case was for very long
> usages, not usage pages.)
Ah, you are right. usage_page should always be 2 bytes, and usage can
be 4 bytes or less.
-t
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
