Nir Soffer has uploaded a new change for review. Change subject: lvm: Do not use udev to set permissions on vdsm images ......................................................................
lvm: Do not use udev to set permissions on vdsm images udev has changed the rules recently, so setting USER, GROUP, or MODE will change also the selinux label behind your back. This issue caused vms to pause after extend, rendering thin provisioning disk useless. We had a temporary fix, applying a static selinux label on vdsm images, but this breaks libvirt security. This patch avoids this issue by not using udev to set any permissions, thus preserving the secure selinux labels set by libvirt. This patch replaces the following commits with a simpler and hopefully longer lasting fix which does not need any platform specific code. d8d6c17 gitignore: Ingore vdsm-lvm.rule.tpl 00fbc83 lvm: Modify lv selinux label only if not labablled as libvirt image b2268e4 spec: Enable lvm selinux fix for Fedora 75fc495 lvm: Set libvirt image selinux label on block devices backing vdsm images Change-Id: I57d9987bf0be19e6e233baaeea10877918eb849b Bug-Url: https://bugzilla.redhat.com/1149883 Signed-off-by: Nir Soffer <[email protected]> --- M .gitignore M configure.ac M vdsm.spec.in M vdsm/storage/Makefile.am D vdsm/storage/vdsm-chcon.in R vdsm/storage/vdsm-lvm.rules.in 6 files changed, 6 insertions(+), 76 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/75/33875/1 diff --git a/.gitignore b/.gitignore index 5328c5f..5890806 100644 --- a/.gitignore +++ b/.gitignore @@ -60,9 +60,7 @@ vdsm/sos/vdsm.py vdsm/storage/protect/safelease vdsm/storage/lvm.env -vdsm/storage/vdsm-chcon vdsm/storage/vdsm-lvm.rules -vdsm/storage/vdsm-lvm.rules.tpl vdsm/sudoers.vdsm vdsm/svdsm.logger.conf vdsm/vdscli.py diff --git a/configure.ac b/configure.ac index 238e519..584b165 100644 --- a/configure.ac +++ b/configure.ac @@ -56,17 +56,6 @@ AM_CONDITIONAL([HOOKS], [test "${enable_hooks}" = "yes"]) AC_ARG_ENABLE( - [chcon_hack], - [AS_HELP_STRING( - [--enable-chcon-hack], - [enable chcon hack for block devices @<:@default=no@:>@] - )], - , - [enable_chcon_hack="no"] -) -AM_CONDITIONAL([CHCON_HACK], [test "${enable_chcon_hack}" = "yes"]) - -AC_ARG_ENABLE( [libvirt-sanlock], [AS_HELP_STRING( [--disable-libvirt-sanlock], @@ -121,11 +110,6 @@ [with_libvirt_service_default="${sysconfdir}/sysconfig/libvirtd"] ) AC_SUBST([LIBVIRT_SERVICE_DEFAULT], ["${with_libvirt_service_default}"]) - - -# Selinux image label -AC_SUBST([SVIRT_IMAGE_LABEL], ['svirt_image_t']) -AC_SUBST([SVIRT_CONTENT_LABEL], ['svirt_content_t']) # Users and groups @@ -262,7 +246,6 @@ AC_PATH_PROG([BLKID_PATH], [blkid], [/sbin/blkid]) AC_PATH_PROG([BRCTL_PATH], [brctl], [/usr/sbin/brctl]) AC_PATH_PROG([CAT_PATH], [cat], [/bin/cat]) -AC_PATH_PROG([CHCON_PATH], [chcon], [/bin/chcon]) AC_PATH_PROG([CHKCONFIG_PATH], [chkconfig], [/sbin/chkconfig]) AC_PATH_PROG([CHMOD_PATH], [chmod], [/bin/chmod]) AC_PATH_PROG([CHOWN_PATH], [chown], [/bin/chown]) @@ -284,7 +267,6 @@ AC_PATH_PROG([IP_PATH], [ip], [/sbin/ip]) AC_PATH_PROG([ISCSIADM_PATH], [iscsiadm], [/sbin/iscsiadm]) AC_PATH_PROG([KILL_PATH], [kill], [/bin/kill]) -AC_PATH_PROG([LS_PATH], [ls], [/bin/ls]) AC_PATH_PROG([LVM_PATH], [lvm], [/sbin/lvm]) AC_PATH_PROG([MKFS_MSDOS_PATH], [mkfs.msdos], [/sbin/mkfs.msdos]) AC_PATH_PROG([MKFS_PATH], [mkfs], [/sbin/mkfs]) @@ -355,8 +337,7 @@ vdsm/storage/Makefile vdsm/storage/imageRepository/Makefile vdsm/storage/protect/Makefile - vdsm/storage/vdsm-chcon - vdsm/storage/vdsm-lvm.rules.tpl + vdsm/storage/vdsm-lvm.rules vdsm/virt/Makefile vdsm_hooks/Makefile vdsm_hooks/checkimages/Makefile diff --git a/vdsm.spec.in b/vdsm.spec.in index c38d5bb..8fc4593 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -40,10 +40,6 @@ %global with_vhostmd 1 %endif -%if 0%{?fedora} >= 19 || 0%{?rhel} >= 7 -%global with_chcon_hack 1 -%endif - %if 0%{?fedora} >= 15 || 0%{?rhel} >= 7 %global with_systemd 1 %endif @@ -663,7 +659,7 @@ %if 0%{?enable_autotools} autoreconf -if %endif -%configure %{?with_hooks:--enable-hooks} %{?with_chcon_hack:--enable-chcon-hack} +%configure %{?with_hooks:--enable-hooks} make # Setting software_version and software_revision in dsaversion.py baserelease=`echo "%{release}" | sed 's/\([0-9]\+\(\.[0-9]\+\)\?\).*/\1/'` @@ -689,11 +685,6 @@ # Install the lvm rules install -Dm 0644 vdsm/storage/vdsm-lvm.rules \ %{buildroot}%{_udevrulesdir}/12-vdsm-lvm.rules - -%if 0%{?with_chcon_hack} -install -Dm 0755 vdsm/storage/vdsm-chcon \ - %{buildroot}%{_udevexecdir}/vdsm-chcon -%endif install -Dm 0644 vdsm/limits.conf \ %{buildroot}/etc/security/limits.d/99-vdsm.conf @@ -1187,9 +1178,6 @@ %endif %{python_sitelib}/sos/plugins/vdsm.py* %{_udevrulesdir}/12-vdsm-lvm.rules -%if 0%{?with_chcon_hack} -%{_udevexecdir}/vdsm-chcon -%endif /etc/security/limits.d/99-vdsm.conf %{_mandir}/man8/vdsmd.8* %if 0%{?rhel} diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am index 89fa1e5..99b1460 100644 --- a/vdsm/storage/Makefile.am +++ b/vdsm/storage/Makefile.am @@ -81,22 +81,3 @@ EXTRA_DIST = \ lvm.env.in \ $(NULL) - -all-local: vdsm-lvm.rules - -vdsm-lvm.rules: vdsm-lvm.rules.tpl -if CHCON_HACK - python -c '\ - import sys, re; \ - s = open(sys.argv[1]).read(); \ - pat = re.compile(r"{{.+?}}\n?", re.S); \ - s = pat.sub("", s); \ - sys.stdout.write(s)' $< > $@; -else - python -c '\ - import sys, re; \ - s = open(sys.argv[1]).read(); \ - pat = re.compile(r"{{if chcon_hack}}\n?.+?{{endif}}\n?", re.S); \ - s = pat.sub("", s); \ - sys.stdout.write(s)' $< > $@; -endif diff --git a/vdsm/storage/vdsm-chcon.in b/vdsm/storage/vdsm-chcon.in deleted file mode 100644 index 6f1eb6e..0000000 --- a/vdsm/storage/vdsm-chcon.in +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/sh - -# This script must be called from a udev rule and assumes the udev environment -# variables. - -# Do not touch the device if it is already labelled is libvirt image. It will -# probably be a fixed_disk_t or it may have no selinux label. -if @LS_PATH@ -Z "$DEVNAME" | \ - @GREP_PATH@ -q -E ":@SVIRT_CONTENT_LABEL@:|:@SVIRT_IMAGE_LABEL@:"; then - exit 0 -fi - -echo "Changing selinux type to @SVIRT_IMAGE_LABEL@ on $DEVNAME" >&2 -@CHCON_PATH@ -t @SVIRT_IMAGE_LABEL@ "$DEVNAME" diff --git a/vdsm/storage/vdsm-lvm.rules.tpl.in b/vdsm/storage/vdsm-lvm.rules.in similarity index 82% rename from vdsm/storage/vdsm-lvm.rules.tpl.in rename to vdsm/storage/vdsm-lvm.rules.in index bdb33f1..34be9a5 100644 --- a/vdsm/storage/vdsm-lvm.rules.tpl.in +++ b/vdsm/storage/vdsm-lvm.rules.in @@ -8,13 +8,6 @@ # # Vdsm rules for lvm logical volumes. -{{if chcon_hack}} -# The libvirt image label is required to allow qemu to access volumes. Libvirt -# sets this label on volumes when starting a vm, but on EL7 and Fedora the -# label is lost after refreshing a logical volume, and vm get paused. This rule -# ensures that the label exist after device changes. See -# https://bugzilla.redhat.com/1147910 -{{endif}} # "add" event is processed on coldplug only, so we need "change", too. ACTION!="add|change", GOTO="lvm_end" @@ -23,7 +16,10 @@ ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end" # Volumes used as vdsm images -ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@"{{if chcon_hack}}, RUN+="vdsm-chcon"{{endif}}, GOTO="lvm_end" +# WARNING: we cannot use USER, GROUP and MODE since using any of them will +# change the selinux label to the default, causing vms to pause after extending +# disks. https://bugzilla.redhat.com/1147910 +ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", RUN+="@CHOWN_PATH@ @VDSMUSER@:@QEMUGROUP@ $env{DEVNAME}", GOTO="lvm_end" # Temprory volumes - not accessed by libvirt/qemu ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]_MERGE", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -- To view, visit http://gerrit.ovirt.org/33875 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I57d9987bf0be19e6e233baaeea10877918eb849b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
