[systemd-devel] [PATCH] udev: Add hidraw_id and a rule file to invoke it

[Tom Gundersen]

On Mon, Nov 3, 2014 at 5:47 PM, Andy Lutomirski <luto at amacapital.net> wrote:
> On Mon, Nov 3, 2014 at 5:32 AM, Tom Gundersen <teg at jklm.no> wrote:
>> Hi Andy,
>>
>> On Tue, Oct 28, 2014 at 11:46 PM, Andy Lutomirski <luto at 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