Comment 5 for bug 1103022

Serge Hallyn [2013-01-29 6:22 -0000]:
> I'm assuming (hoping) comment #2 as entered before we proceeded with our
> irc conversation?

Yes, I did after the first part when it seemed to me that we agreed
that this was only an upgrade fix for the wrong
40-qemu-system.rules.

> 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"?

> 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
/dev/kvm, the kernel default is 600 root:root. 70-udev-acl.rules
merely tags it with UDEV_ACL, which causes udev-acl.ck to add
additional user ACLs for the user currently owning the foreground
console.

> 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
  $ echo 'KERNEL=="kvm", GROUP="kvm", MODE="0660"' | sudo tee /lib/udev/rules.d/40-qemu-system.rule
  # switch back to VT1
  # ls -l /dev/kvm; getfacl /dev/kvm
  crw-rw---- 1 root kvm 10, 232 Jan 29 08:46 /dev/kvm
  getfacl: Entferne führende '/' von absoluten Pfadnamen
  # file: dev/kvm
  # owner: root
  # group: kvm
  user::rw-
  group::rw-
  other::---

Everything as expected. Now switch back to desktop session:

  $ ls -l /dev/kvm; getfacl /dev/kvm
  crw-rw----+ 1 root kvm 10, 232 Jan 29 08:46 /dev/kvm
  # file: dev/kvm
  # owner: root
  # group: kvm
  user::rw-
  user:martin:rw-
  group::rw-
  mask::rw-
  other::---

Still everything as expected.

So I'm still convinced that 40-qemu-system.rule is the only problem
here, and that it should be replaced with something like

  KERNEL=="kvm", GROUP="kvm", MODE="0660"

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