ACL_MANAGE meaning in udev is inconsistent

ACL_MANAGE meaning in udev is inconsistent

[Rafał Rzepecki]

[This was first submitted as
http://bugzilla.kernel.org/show_bug.cgi?id\x15353 and rejected
explaining the bugzilla is for kernel bugs only and I should contact
this list. Below is the complete bugreport as submitted.
I'd appreciate CC on any follow-ups.]

ACL_MANAGE is an udev property meant to mark devices which permissions that
should be dynamically managed with ACLs. udev-acl updates permissions when
adding a device, and, in collaboration with ConsoleKit, on user console login
and logout.

In extras/udev-acl/70-acl.rules:4 ACL_MANAGE is tested for zeroness to check
whether a device should be excluded from ACL management:

ENV{ACL_MANAGE}="0", GOTO="acl_end"

This is not consistent with usage in extras/udev-acl/udev-acl.c:292:

udev_enumerate_add_match_property(enumerate, "ACL_MANAGE", "*");

which checks merely for the property's nonemptiness. This means that if
ACL_MANAGE is set in system-local rules to "0", initially device's ACL will be
left alone (70-acl.rules will see "0" and skip calling udev-acl), but when
ConsoleKit subsequently calls udev-acl on user login, the permissions will be
modified (as ACL_MANAGE="0" is nonempty).

To selectively disable ACL management for a device one has currently resort to
a workaround of placing a local rule setting ACL_MANAGE="0" before
70-acl.rules, and another rule setting ACL_MANAGE="" after it.

To fix it, extras/udev-acl/udev-acl.c:292 should be changed to read
udev_enumerate_add_match_property(enumerate, "ACL_MANAGE", "1");

-- 
Rafał Rzepecki
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ACL_MANAGE meaning in udev is inconsistent

[Martin Pitt]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

Hey Rafał,

Rafał Rzepecki [2010-02-19  7:37 +0100]:
> In extras/udev-acl/70-acl.rules:4 ACL_MANAGE is tested for zeroness to check
> whether a device should be excluded from ACL management:
> 
> ENV{ACL_MANAGE}=="0", GOTO="acl_end"
> 
> This is not consistent with usage in extras/udev-acl/udev-acl.c:292:

Thanks for catching this! Fixed in

  http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff;h=5c3ebbf35a2c101e0212c7066f0d65e457fcf40c

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: ACL_MANAGE meaning in udev is inconsistent

[Kay Sievers]

2010/3/3 Martin Pitt <martin.pitt@ubuntu.com>:
> Rafał Rzepecki [2010-02-19  7:37 +0100]:
>> In extras/udev-acl/70-acl.rules:4 ACL_MANAGE is tested for zeroness to check
>> whether a device should be excluded from ACL management:
>>
>> ENV{ACL_MANAGE}="0", GOTO="acl_end"
>>
>> This is not consistent with usage in extras/udev-acl/udev-acl.c:292:
>
> Thanks for catching this! Fixed in
>
>  http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff;h\3ebbf35a2c101e0212c7066f0d65e457fcf40c

I'll remove that later on. Please do not rely on this functionality.
The ACL_MANAGE key must not leak to any other rules file. We will
likely change that name to be able to speed things up, and nothing
should use it outside the acl rules file.

Thanks,
Kay

Re: ACL_MANAGE meaning in udev is inconsistent

[Frederic Crozat]

Le 04/03/2010 01:33, Kay Sievers a écrit :
> 2010/3/3 Martin Pitt<martin.pitt@ubuntu.com>:
>> Rafał Rzepecki [2010-02-19  7:37 +0100]:
>>> In extras/udev-acl/70-acl.rules:4 ACL_MANAGE is tested for zeroness to check
>>> whether a device should be excluded from ACL management:
>>>
>>> ENV{ACL_MANAGE}="0", GOTO="acl_end"
>>>
>>> This is not consistent with usage in extras/udev-acl/udev-acl.c:292:
>>
>> Thanks for catching this! Fixed in
>>
>>   http://git.kernel.org/?p=linux/hotplug/udev.git;a=commitdiff;h\3ebbf35a2c101e0212c7066f0d65e457fcf40c
>
> I'll remove that later on. Please do not rely on this functionality.
> The ACL_MANAGE key must not leak to any other rules file. We will
> likely change that name to be able to speed things up, and nothing
> should use it outside the acl rules file.

Hmm, this is problematic : a lot of rules outside udev are using 
ACL_MANAGE to specify ACL should be set for some devices. I would 
suggest to not "deprecate" ACL_MANAGE outside 70-acl.rules but 
internally to 70-acl.rules, use another variable.

-- 
Frederic Crozat
Mandriva

Re: ACL_MANAGE meaning in udev is inconsistent

[Martin Pitt]

Frederic Crozat [2010-03-04  9:40 +0100]:
> Hmm, this is problematic : a lot of rules outside udev are using
> ACL_MANAGE to specify ACL should be set for some devices. I would
> suggest to not "deprecate" ACL_MANAGE outside 70-acl.rules but
> internally to 70-acl.rules, use another variable.

Oh, do you know which? The only one I'm aware of is gnome-bluetooth
for /dev/rfkill, which is evil, bad, and wrong:

  https://bugzilla.redhat.com/show_bug.cgi?idQ4798

It really shouldn't be used outside of 70-acl.rules.

Kay, perhaps we should turn this into a non-public symbol, like
"_ACL_MANAGE"?

Martin

Martin
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)

Re: ACL_MANAGE meaning in udev is inconsistent

[Frederic Crozat]

Le 04/03/2010 10:26, Martin Pitt a écrit :
> Frederic Crozat [2010-03-04  9:40 +0100]:
>> Hmm, this is problematic : a lot of rules outside udev are using
>> ACL_MANAGE to specify ACL should be set for some devices. I would
>> suggest to not "deprecate" ACL_MANAGE outside 70-acl.rules but
>> internally to 70-acl.rules, use another variable.
>
> Oh, do you know which? The only one I'm aware of is gnome-bluetooth
> for /dev/rfkill, which is evil, bad, and wrong:
>
>    https://bugzilla.redhat.com/show_bug.cgi?idQ4798
>
> It really shouldn't be used outside of 70-acl.rules.

For a quick check on my test system :

/etc/udev/rules.d/60-libpisock.rules
needed for Palm devices

/etc/udev/rules.d/69-floppy-acl.rules
needed for mtools

/etc/udev/rules.d/69-gammu-acl.rules
needed for gammu

/lib/udev/rules.d/60-libmtp.rules
needed for MTP devices

/lib/udev/rules.d/69-printeracl.rules
for parallel printers (not sure it is still needed these days)

/etc/udev/rules.d/55-Argyll.rules:
needed for colorimeters (this file is supposed to be used across various 
udev releases and distribution, even if COLOR_MEASUREMENT_DEVICE is now 
merged in udev upstream : I've removed this specific ACL_MANAGE in our 
Mandriva package)

/lib/udev/rules.d/95-gcm-colorimeters.rules
for gnome-color-manager : it kind of duplicate some of the work done in 
argyllcms rules, because you might not have argyllcms installed 
initially (Richard is going to rename the rule which is currently 
executed after 70-acl which is a bug).

I might have missed other rules, I didn't do a full search across our 
package database.

-- 
Frederic Crozat
Mandriva

Re: ACL_MANAGE meaning in udev is inconsistent

[Kay Sievers]

On Wed, Mar 3, 2010 at 23:26, Martin Pitt <martin.pitt@ubuntu.com> wrote:
> Frederic Crozat [2010-03-04  9:40 +0100]:
>> Hmm, this is problematic : a lot of rules outside udev are using
>> ACL_MANAGE to specify ACL should be set for some devices. I would
>> suggest to not "deprecate" ACL_MANAGE outside 70-acl.rules but
>> internally to 70-acl.rules, use another variable.
>
> Oh, do you know which? The only one I'm aware of is gnome-bluetooth
> for /dev/rfkill, which is evil, bad, and wrong:
>
>  https://bugzilla.redhat.com/show_bug.cgi?idQ4798
>
> It really shouldn't be used outside of 70-acl.rules.
>
> Kay, perhaps we should turn this into a non-public symbol, like
> "_ACL_MANAGE"?

We need to find it in the db at coldplug time. :)

If we are not getting a better idea, I'm planing to add a TAGfeature, which maintains an index database which supports a
reverse-lookup. Also for other use-cases, we need a more efficient way
to select a set of devices tagged by rules. At this time ACL_MANAGE
will go away.

Thanks,
Kay

Re: ACL_MANAGE meaning in udev is inconsistent

[Kay Sievers]

On Wed, Mar 3, 2010 at 22:40, Frederic Crozat <fcrozat@mandriva.com> wrote:
> Le 04/03/2010 01:33, Kay Sievers a écrit :

>> I'll remove that later on. Please do not rely on this functionality.
>> The ACL_MANAGE key must not leak to any other rules file. We will
>> likely change that name to be able to speed things up, and nothing
>> should use it outside the acl rules file.
>
> Hmm, this is problematic : a lot of rules outside udev are using ACL_MANAGE
> to specify ACL should be set for some devices. I would suggest to not
> "deprecate" ACL_MANAGE outside 70-acl.rules but internally to 70-acl.rules,
> use another variable.

Then please tag these devices with some other useful classification
and add that match to the acl rules file. If things happen as planned,
ACL_MANAGE will go away.

Thanks,
Kay