On Thu, Sep 22, 2022 at 13:40:43 +0200, Michal Privoznik wrote:
For NVMe disks we skip setting SELinux label on corresponding
VFIO group (/dev/vfio/X). This bug is the best visible with
best visible? or only visible?
namespaces and goes as follows:
1) libvirt assigns NVMe disk to vfio-pci driver,
2) kernel creates /dev/vfio/X node with generic device_t SELinux
label,
3) our namespace code creates the exact copy of the node in
domain's private /dev,
4) SELinux policy kicks in an changes the label on the node to
vfio_device_t (in the top most namespace),
Seems like this would prevent that bug without namespaces.
5) libvirt tells QEMU to attach the NVMe disk, which is denied by
SELinux policy.
While one can argue that kernel should have created the
/dev/vfio/X node with the correct SELinux label from the
beginning (step 2), libvirt can't rely on that and needs to set
label on its own.
Surprisingly, I already wrote the code that aims on this specific
case (v6.0.0-rc1~241), but because of a shortcut we take earlier
it is never ran. The reason is that
virStorageSourceIsLocalStorage() considers NVMe disks as
non-local because their source is not accessible via src->path
(or even if it is, it's not a local path).
Therefore, do not exit early for NVMe disks and let the function
continue.
Resolves:
https://bugzilla.redhat.com/show_bug.cgi?id=2121441
Fixes: 284a12bae0e4cf93ea72797965d6c12e3a103f40
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/security/security_selinux.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 9f2872decc..a296cb7613 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1818,7 +1818,11 @@ virSecuritySELinuxSetImageLabelInternal(virSecurityManager *mgr,
const char *path = src->path;
int ret;
- if (!src->path || !virStorageSourceIsLocalStorage(src))
+ /* Special case NVMe. Per virStorageSourceIsLocalStorage() it's
+ * considered not local, but we still want the code below to set
+ * label on VFIO group. */
This comment gets deleted right in the next patch. Consider just
omitting it completely.
+ if (src->type != VIR_STORAGE_TYPE_NVME &&
+ (!src->path || !virStorageSourceIsLocalStorage(src)))
return 0;
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>