[PATCH] qemu: do not allow /dev/rtc or /dev/hpet access via the devices cgroup

The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 years ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e. Do not allow them in the devices cgroup policy. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/drvqemu.html.in | 1 - src/qemu/qemu.conf | 1 - src/qemu/qemu_cgroup.c | 1 - src/qemu/test_libvirtd_qemu.aug.in | 2 -- 4 files changed, 5 deletions(-) diff --git a/docs/drvqemu.html.in b/docs/drvqemu.html.in index afc4ddf56d..b6d731bb59 100644 --- a/docs/drvqemu.html.in +++ b/docs/drvqemu.html.in @@ -484,7 +484,6 @@ chmod o+x /path/to/directory /dev/null, /dev/full, /dev/zero, /dev/random, /dev/urandom, /dev/ptmx, /dev/kvm, -/dev/rtc, /dev/hpet </pre> <p> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index abdbf07fec..d7a3f40e78 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -495,7 +495,6 @@ # "/dev/null", "/dev/full", "/dev/zero", # "/dev/random", "/dev/urandom", # "/dev/ptmx", "/dev/kvm", -# "/dev/rtc","/dev/hpet" #] # # RDMA migration requires the following extra files to be added to the list: diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 2e019b64af..d92202f847 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -47,7 +47,6 @@ const char *const defaultDeviceACL[] = { "/dev/null", "/dev/full", "/dev/zero", "/dev/random", "/dev/urandom", "/dev/ptmx", "/dev/kvm", - "/dev/rtc", "/dev/hpet", NULL, }; #define DEVICE_PTY_MAJOR 136 diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 19da591aae..e533b9f551 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -61,8 +61,6 @@ module Test_libvirtd_qemu = { "5" = "/dev/urandom" } { "6" = "/dev/ptmx" } { "7" = "/dev/kvm" } - { "8" = "/dev/rtc" } - { "9" = "/dev/hpet" } } { "save_image_format" = "raw" } { "dump_image_format" = "raw" } -- 2.25.4

On 5/19/20 1:06 AM, Paolo Bonzini wrote:
The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 years ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e. Do not allow them in the
qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e v0.14.0-rc0-1169-g25f3151ece and the minimum supported version is 1.5.0 so this is safe to merge from min version POV.
devices cgroup policy.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/drvqemu.html.in | 1 - src/qemu/qemu.conf | 1 - src/qemu/qemu_cgroup.c | 1 - src/qemu/test_libvirtd_qemu.aug.in | 2 -- 4 files changed, 5 deletions(-)
It's not only QEMU that might use these but also a library that is linking with. However, quick strace showed no access to either of the files so: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> And pushed. Michal

On Tue, May 19, 2020 at 10:10:54AM +0200, Michal Privoznik wrote:
On 5/19/20 1:06 AM, Paolo Bonzini wrote:
The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 years ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e. Do not allow them in the
qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e v0.14.0-rc0-1169-g25f3151ece
and the minimum supported version is 1.5.0 so this is safe to merge from min version POV.
devices cgroup policy.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/drvqemu.html.in | 1 - src/qemu/qemu.conf | 1 - src/qemu/qemu_cgroup.c | 1 - src/qemu/test_libvirtd_qemu.aug.in | 2 -- 4 files changed, 5 deletions(-)
It's not only QEMU that might use these but also a library that is linking with. However, quick strace showed no access to either of the files so:
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
And pushed.
This broke make check https://ci.centos.org/view/libvirt/job/libvirt-check/systems=libvirt-fedora-... though I don't understand why as it looks like it removed all the right pieces. I wonder if we had a bad dependancy in make rules meaning we didn't regenerate Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 5/19/20 10:55 AM, Daniel P. Berrangé wrote:
On Tue, May 19, 2020 at 10:10:54AM +0200, Michal Privoznik wrote:
On 5/19/20 1:06 AM, Paolo Bonzini wrote:
The RTC and HPET modes for the QEMU emulation tick have been dropped almost 9 years ago, in commit 25f3151ece1d5881826232bebccc21b588d4e03e. Do not allow them in the
qemu.git $ git describe --tags 25f3151ece1d5881826232bebccc21b588d4e03e v0.14.0-rc0-1169-g25f3151ece
and the minimum supported version is 1.5.0 so this is safe to merge from min version POV.
devices cgroup policy.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- docs/drvqemu.html.in | 1 - src/qemu/qemu.conf | 1 - src/qemu/qemu_cgroup.c | 1 - src/qemu/test_libvirtd_qemu.aug.in | 2 -- 4 files changed, 5 deletions(-)
It's not only QEMU that might use these but also a library that is linking with. However, quick strace showed no access to either of the files so:
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
And pushed.
This broke make check
https://ci.centos.org/view/libvirt/job/libvirt-check/systems=libvirt-fedora-...
though I don't understand why as it looks like it removed all the right pieces. I wonder if we had a bad dependancy in make rules meaning we didn't regenerate
Ah, could it be because of the stray comma? From qemu.conf: #cgroup_device_acl = [ # "/dev/null", "/dev/full", "/dev/zero", # "/dev/random", "/dev/urandom", # "/dev/ptmx", "/dev/kvm", #] Let me check if removing the comma after /dev/kvm fixes the build. Michal

On 19/05/20 10:58, Michal Privoznik wrote:
Ah, could it be because of the stray comma? From qemu.conf:
#cgroup_device_acl = [ # "/dev/null", "/dev/full", "/dev/zero", # "/dev/random", "/dev/urandom", # "/dev/ptmx", "/dev/kvm", #]
Let me check if removing the comma after /dev/kvm fixes the build.
It does. I forgot to commit it, sorry. :( Though perhaps accepting the trailing comma is better, which would be something like the following untested patch: diff --git a/src/bhyve/libvirtd_bhyve.aug b/src/bhyve/libvirtd_bhyve.aug index 66079376c4..9f4e582ab9 100644 --- a/src/bhyve/libvirtd_bhyve.aug +++ b/src/bhyve/libvirtd_bhyve.aug @@ -15,7 +15,7 @@ module Libvirtd_bhyve = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] diff --git a/src/libxl/libvirtd_libxl.aug b/src/libxl/libvirtd_libxl.aug index 58b9af3707..39049cf139 100644 --- a/src/libxl/libvirtd_libxl.aug +++ b/src/libxl/libvirtd_libxl.aug @@ -15,7 +15,7 @@ module Libvirtd_libxl = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] diff --git a/src/locking/virtlockd.aug b/src/locking/virtlockd.aug index 06d508e6e5..66eb4125ad 100644 --- a/src/locking/virtlockd.aug +++ b/src/locking/virtlockd.aug @@ -15,7 +15,7 @@ module Virtlockd = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] diff --git a/src/logging/virtlogd.aug b/src/logging/virtlogd.aug index 04580734d6..60bbf44305 100644 --- a/src/logging/virtlogd.aug +++ b/src/logging/virtlogd.aug @@ -15,7 +15,7 @@ module Virtlogd = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] diff --git a/src/lxc/libvirtd_lxc.aug b/src/lxc/libvirtd_lxc.aug index be6402cc01..e6ab5f0dde 100644 --- a/src/lxc/libvirtd_lxc.aug +++ b/src/lxc/libvirtd_lxc.aug @@ -15,7 +15,7 @@ module Libvirtd_lxc = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index 404498b611..aceace7d86 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -15,7 +15,7 @@ module Libvirtd_qemu = let bool_val = store /0|1/ let int_val = store /[0-9]+/ let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" - let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + let str_array_val = counter "el" . array_start . ( ( str_array_element . array_sep ) * . str_array_element ? ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ]
participants (3)
-
Daniel P. Berrangé
-
Michal Privoznik
-
Paolo Bonzini