Comment 6 for bug 1103022

Revision history for this message
Serge Hallyn (serge-hallyn) wrote : Re: [Bug 1103022] Re: 70-udev-acl.rules needs to put g+rw on /dev/kvm

> > The 40-qemu-system.rules is not the problem and we're not trying to recover from it.
>
> Well, it is a problem in the sense that the setfacl call there should
> not be necessary and also should be redundant, as it is essentially
> equivalent to "chmod g+rw"?

I'd love to remove it (I only very recently added it), but it is not
redundant with chmod g+rw, as chmod does not remove acls.

> > 1. set up a new ubuntu system, it modprobes kvm_intel, /dev/kvm gets created,
> > 2. 70-udev-acl.rules sets /dev/kvm to root:root rwx------, and tags it with acl
>
> No, 70-udev-acl.rules does not change the ownership or permissions of

Ok, fine, I'll change that to "leaves it root:root rwx------ when it
could otherwise change it to root:root rwxrw----".

> > 3. user logs in, something (consolekit?) adds a group::--- acl
>
> That's the bit which I don't understand. I seriously doubt that this
> is udev-acl, as that only adds additional user ACLs. What makes you
> sure that this is actually an ACL, as opposed to just being the
> ACL representation of the original "600" permissions that /dev/kvm
> gets without 40-qemu-system.rules?
>
> - The udev-acl.ck code does not look like it would be able to do it
> - As far as I know, ACLs are additive to Unix file permissions, i. e.
> they can never remove access.
> - I don't see any call to *facl in any installed postinst script.
>
> > 4. admin logs in remotely, installs qemu-system and libvirt-bin, which triggers udev with new rules,
> > 5. udev chowns /dev/kvm to root:kvm, and sets it to rwxrw----, but the group::--- acl remains
> > [...]
> > As discussed on irc, I'll see if I can figure out what exactly is causing that
> > group acl to be (needlessly) written.
>
> Right, that seems to be the crux of the problem.
>
> I tried to reproduce this:
>
> $ sudo mv /lib/udev/rules.d/40-qemu-system.rules{,.disabled}
> $ sudo rmmod kvm_intel kvm
> $ sudo passwd root
> # log into VT1 as root, to avoid having a CK session
> # modprobe kvm_intel
> # ls -l /dev/kvm; getfacl /dev/kvm
> crw------- 1 root root 10, 232 Jan 29 08:36 /dev/kvm
> # file: dev/kvm
> # owner: root
> # group: root
> user::rw-
> group::---
> other::---
>
> So far so good. We should be after step 2 now. Now I switch back to my
> desktop session, so that 70-udev-acl.rules kicks in:
>
> $ ls -l /dev/kvm; getfacl /dev/kvm
> crw-rw----+ 1 root root 10, 232 Jan 29 08:36 /dev/kvm
> getfacl: Entferne führende '/' von absoluten Pfadnamen
> # file: dev/kvm
> # owner: root
> # group: root
> user::rw-
> user:martin:rw-
> group::---
> mask::rw-
> other::---
>
> We see that udev added the dynamic ACL. That's step 3. Now let's
> "install" the new rules:
>
> $ sudo mv /lib/udev/rules.d/40-qemu-system.rules{.disabled,}

[...]

> $ udevadm trigger --subsystem-match=misc --action=change --verbose
> [...]
> /sys/devices/virtual/misc/kvm
>
> $ ls -l /dev/kvm; getfacl /dev/kvm
> crw-rw----+ 1 root root 10, 232 Jan 29 08:36 /dev/kvm
> # file: dev/kvm
> # owner: root
> # group: root
> user::rw-
> user:martin:rw-
> group::---
> mask::rw-
> other::---
>
> And that's indeed where things go haywire, the group is overwritten to
> "root" because of /40-qemu-system.rules. So let's replace this with a
> rule as it should be:
>
> $ sudo rmmod kvm_intel kvm

This is where you've gone wrong in your experiment. The
/dev/kvm perms will be correct because you've reloaded kvm_intel
after a (any) 40-qemu-system.rule existed.