[Bug 1103022] Re: 70-udev-acl.rules needs to put g+rw on /dev/kvm

[Martin Pitt]

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)

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to udev in Ubuntu.
https://bugs.launchpad.net/bugs/1103022

Title:
  70-udev-acl.rules needs to put g+rw on /dev/kvm

Status in “udev” package in Ubuntu:
  Confirmed

Bug description:
  When qemu-system gets installed, the newly installed udev rule causes
  /dev/kvm to gets chgrpd to kvm and its mode to get set to g+rw.
  However, because /dev/kvm was tagged with ACL previously, there is a
  group:: acl on /dev/kvm which does not get removed.  Therefore
  /dev/kvm is g+rw in the file mode, but the acl denies group read/write
  access.  After a reboot all is fine.

  I have not seen a clean way to have udev remove that acl, and there is
  no reason for it.  So please update the 70-udev-acl.rules file to set
  MODE=0660 on /dev/kvm

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/udev/+bug/1103022/+subscriptions