Bug 665 - udev rules use obsolete 'plugdev' group
Summary: udev rules use obsolete 'plugdev' group
Status: RESOLVED FIXED
Alias: None
Product: libsigrok
Classification: Unclassified
Component: Portability (show other bugs)
Version: unreleased development snapshot
Hardware: All All
: Normal normal
Target Milestone: ---
Assignee: Nobody
URL: https://bugzilla.redhat.com/show_bug....
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-10 03:18 CEST by mrnuke
Modified: 2018-11-17 17:43 CET (History)
3 users (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description mrnuke 2015-10-10 03:18:14 CEST
According to the Fedora bug report [1],

libsigrok uses the obsolete group mechanism in udev rules: assigns device files to be R/W by group plugdev, which does not exist on Fedora. The correct action is to use systemd session mechanism that tags the devices as 'uaccess', which then creates ACLs enabling R/W for the current logged-in console user.

Personally, I've been using libsigrok succesfully on the latest Fedora, and haven't noticed an issue. I leave the matter to you gentlemen to decide how you wish to proceed.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1270102
Comment 1 Martin Ling 2016-01-04 00:13:53 CET
It's absolutely appropriate for Fedora to adapt the example udev rules file as needed for that distribution.

I don't see a libsigrok bug here: udev actions needed are distro-specific so the file can only ever be an example. If we change the upstream file to suit Fedora it will be just as wrong on some other distribution.
Comment 2 mrnuke 2016-01-04 02:46:49 CET
I'm not going to argue with the reasoning, but it seems that the "uaccess" tag is the generic way to handle this, so I don't think closing this bug as "INVALID" is appropriate.
Comment 3 Martin Ling 2016-01-04 10:21:31 CET
(In reply to comment #2)
> I'm not going to argue with the reasoning, but it seems that the "uaccess"
> tag is the generic way to handle this, so I don't think closing this bug as
> "INVALID" is appropriate.

I was going by the comments on the Fedora bug that it's systemd specific at least.
Comment 4 Martin Ling 2016-01-04 12:51:42 CET
After some consideration I was going to come back and reopen this because of comments from others, but I see now that you already did so at the time of your last comment.

To be quite honest though this whole situation is highly frustrating.

You say that this 'uaccess' is the new generic way of doing things, but there's absolutely no documentation about this idea anywhere, not even from Fedora who are now apparently enforcing it. Which other distributions will it work on? How are we supposed to know?

The 'plugdev' group is described as a 'Ubuntuism' in the related Fedora bugs, but it is present and working on my Debian stable system, and apparently also for you on Fedora.

If we change this file to suit Fedora and then get a bug report from a Ubuntu user, what are we supposed to do?

It can't be upstream's job to provide policy-compliant integration with every distribution's policies. That's *exactly* the role of each distribution's package maintainers.

Asking upstream to take distro-specific patches on a say-so, with comments like "but everybody uses systemd now, right?" is self-centred, lazy and quite frankly just rude.

I am leaving the bug open because I accept that we might be able to do something better here, but just pushing the Fedora patches upstream is not the answer.
Comment 5 mrnuke 2016-01-04 19:10:56 CET
I'm not too happy about the lack of documentation either, so I did some digging about plugdev. From what I was able to find out, "plugdev" is a group specific to debian/ubuntu. Searches for "<distro name> plugdev" usually turn up posts that say plugdev group does not exist.


There was a useful post [1] about which tags are appropriate. Seems that "udev-acl" was used on older ubuntu versions, so people suggest using both tags.
Hope this helps.

[1] https://wiki.archlinux.org/index.php/Talk:Udev#Use_of_.27uaccess.27_instead_of_GROUP_and_MODE.3F
Comment 6 Martin Ling 2016-01-04 19:21:41 CET
That only confuses the matter further, especially given that udev releases which used that group had the explicit documentation:

# Do not use TAG+="udev-acl" outside of this file. This variable is private to
# udev-acl of this udev release and may be replaced at any time.

I think my preference at this point is to remove the udev file entirely, and just include a text list of VID/PID pairs that we're interested in. People can generate their own files to suit their distro.
Comment 7 mrnuke 2016-01-04 19:32:31 CET
I can't find the exact link, but IIRC, the reason for having "udev-acl" in addition to "uaccess" was a specific version of ubuntu that didn't understand "uaccess".

If you do decide to go the route of a text file with USB IDs, I'd be more than happy to write a script to parse that and turn it into a rules file.
Comment 8 Martin Ling 2016-01-04 20:31:06 CET
I'm leaving the decision on this up to Uwe, but in case the text file approach is desired I have implemented it on this branch:

https://github.com/martinling/libsigrok/commits/remove-udev-rules
Comment 9 Martin Ling 2016-01-05 13:55:21 CET
And a script is now available on this branch:

https://github.com/s09bQ5/libsigrok/commits/remove-udev-rules
Comment 10 Uwe Hermann 2017-03-29 12:00:46 CEST
All of this has various pros and cons and points to consider wrt portability, simple distro and user handling, simple handling on the sigrok side, etc. See also latest discussion on the mailing list:

  https://www.mail-archive.com/sigrok-devel@lists.sourceforge.net/msg02446.html

I've decided to fix this in the following way, which should address most issues:

 - libsigrok keeps the explicit udev rules file, but TAG+="uaccess" is added additionally (and the "plugdev" group is kept as well). This is supposed to work for most systemd-based distros as well as most non-systemd-based distros. Any further per-distro customizations should be performed by the distro package maintainers as usual. This is easy enough to do with some sed magic, see mailing list posts above.

 - Instead of having a extra plain VID/PID list file, we use a simple Makefile target in sigrok-androidutils that allows us to manually update the device_filter.xml file based on the latest git version of the actual libsigrok udev rules file. Extracting the VIDs/PIDs from there is just as easy as getting them from an extra file which contains just the VIDs/PIDs. That Makefile target can be run manually from time to time (plus a commit of the result) and/or it can also be run programmatically (e.g. in a Jenkins job) if Internet access during the build is not a problem.

I'm closing this bug as fixed, but I wouldn't mind further improvements of this stuff in add-on patches; e.g. the proposal to use ENV{libsigrok_matched}="yes" to simplify the file a bit more could be neat. Patches welcome!

  https://www.mail-archive.com/sigrok-devel@lists.sourceforge.net/msg02458.html

Fixed in libsigrok via:
7a977433368f5d06b584bd7eb80456dabd00359a
68fefbd75c6fd7a27b96c7a979870547fb7ad5ff
3474ec2361148729736c0a104d03330eff21970e

Fixed in sigrok-androidutils via:
7a810032edefa2f6c0e711d9b8be042d21b722c0
Comment 11 Guido Trentalancia 2018-11-17 17:43:57 CET
Please note that the "uaccess" mechanism does not work across all udev versions, see bug #1329:

https://sigrok.org/bugzilla/show_bug.cgi?id=1329

Furthermore, the "uaccess" method does not provide any benfit in this circumstance, see:

https://sigrok.org/bugzilla/show_bug.cgi?id=1329#c0

and even more importantly:

https://sigrok.org/bugzilla/show_bug.cgi?id=1329#c1