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

[Andy Lutomirski]

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.)

[...]

--Andy