File 2e8ebfe3-label-vhostuser-port.patch of Package libvirt.22291

Overview Repositories Revisions Requests Users Attributes Meta

File 2e8ebfe3-label-vhostuser-port.patch of Package libvirt.22291

commit 2e8ebfe3fa17bb785d6bc26a17391dc4f34ad8b3
Author: Jim Fehlig <jfehlig@suse.com>
Date:   Tue Jul 27 18:13:36 2021 -0600

qemu: Set label on vhostuser net device when hotplugging
    
    Attaching a newly created vhostuser port to a VM fails due to an
    apparmor denial
    
    internal error: unable to execute QEMU command 'chardev-add': Failed
    to bind socket to /run/openvswitch/vhu838c4d29-c9: Permission denied
    
    In the case of a net device type VIR_DOMAIN_NET_TYPE_VHOSTUSER, the
    underlying chardev is not labeled in qemuDomainAttachNetDevice prior
    to calling qemuMonitorAttachCharDev.
    
    A simple fix would be to call qemuSecuritySetChardevLabel using the
    embedded virDomainChrSourceDef in the virDomainNetDef vhostuser data,
    but this incurs the risk of incorrectly restoring the label. E.g.
    consider the DAC driver behavior with a vhostuser net device, which
    uses a socket for the chardev backend. The DAC driver uses XATTRS to
    store original labelling information, but XATTRS are not compatible
    with sockets. Without the original labelling information, the socket
    labels will be restored with root ownership, preventing other
    less-privileged processes from connecting to the socket.
    
    This patch avoids overloading chardev labelling with vhostuser net
    devices by introducing virSecurityManager{Set,Restore}NetdevLabel,
    which is currently only implemented for the apparmor driver. The
    new APIs are then used to set and restore labels for the vhostuser
    net devices.
    
    Signed-off-by: Jim Fehlig <jfehlig@suse.com>
    Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Index: libvirt-6.0.0/src/libvirt_private.syms
===================================================================
--- libvirt-6.0.0.orig/src/libvirt_private.syms
+++ libvirt-6.0.0/src/libvirt_private.syms
@@ -1486,6 +1486,7 @@ virSecurityManagerRestoreHostdevLabel;
 virSecurityManagerRestoreImageLabel;
 virSecurityManagerRestoreInputLabel;
 virSecurityManagerRestoreMemoryLabel;
+virSecurityManagerRestoreNetdevLabel;
 virSecurityManagerRestoreSavedStateLabel;
 virSecurityManagerRestoreTPMLabels;
 virSecurityManagerSetAllLabel;
@@ -1497,6 +1498,7 @@ virSecurityManagerSetImageFDLabel;
 virSecurityManagerSetImageLabel;
 virSecurityManagerSetInputLabel;
 virSecurityManagerSetMemoryLabel;
+virSecurityManagerSetNetdevLabel;
 virSecurityManagerSetProcessLabel;
 virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
Index: libvirt-6.0.0/src/qemu/qemu_hotplug.c
===================================================================
--- libvirt-6.0.0.orig/src/qemu/qemu_hotplug.c
+++ libvirt-6.0.0/src/qemu/qemu_hotplug.c
@@ -1156,6 +1156,7 @@ qemuDomainAttachNetDevice(virQEMUDriverP
     g_autofree char *netdev_name = NULL;
     g_autoptr(virConnect) conn = NULL;
     virErrorPtr save_err = NULL;
+    bool teardownlabel = false;
 
     /* preallocate new slot for device */
     if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
@@ -1288,6 +1289,9 @@ qemuDomainAttachNetDevice(virQEMUDriverP
                                                    &net->ifname) < 0)
             goto cleanup;
 
+        if (qemuSecuritySetNetdevLabel(driver, vm, net) < 0)
+            goto cleanup;
+        teardownlabel = true;
         break;
 
     case VIR_DOMAIN_NET_TYPE_USER:
@@ -1471,6 +1475,10 @@ qemuDomainAttachNetDevice(virQEMUDriverP
             qemuDomainNetDeviceVportRemove(net);
         }
 
+        if (teardownlabel &&
+            qemuSecurityRestoreNetdevLabel(driver, vm, net) < 0)
+            VIR_WARN("Unable to restore network device labelling on hotplug fail");
+
         virDomainNetRemoveHostdev(vm->def, net);
 
         if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
@@ -4604,6 +4612,11 @@ qemuDomainRemoveNetDevice(virQEMUDriverP
                          cfg->stateDir));
     }
 
+    if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+        if (qemuSecurityRestoreNetdevLabel(driver, vm, net) < 0)
+            VIR_WARN("Unable to restore security label on vhostuser char device");
+    }
+
     qemuDomainNetDeviceVportRemove(net);
 
     if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
Index: libvirt-6.0.0/src/qemu/qemu_security.c
===================================================================
--- libvirt-6.0.0.orig/src/qemu/qemu_security.c
+++ libvirt-6.0.0/src/qemu/qemu_security.c
@@ -434,6 +434,65 @@ qemuSecurityRestoreChardevLabel(virQEMUD
     return ret;
 }
 
+int
+qemuSecuritySetNetdevLabel(virQEMUDriverPtr driver,
+                           virDomainObjPtr vm,
+                           virDomainNetDefPtr net)
+{
+    int ret = -1;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    pid_t pid = -1;
+
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
+        pid = vm->pid;
+
+    if (virSecurityManagerTransactionStart(driver->securityManager) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerSetNetdevLabel(driver->securityManager,
+                                         vm->def, net) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    virSecurityManagerTransactionAbort(driver->securityManager);
+    return ret;
+}
+
+
+int
+qemuSecurityRestoreNetdevLabel(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm,
+                               virDomainNetDefPtr net)
+{
+    int ret = -1;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    pid_t pid = -1;
+
+    if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
+        pid = vm->pid;
+
+    if (virSecurityManagerTransactionStart(driver->securityManager) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerRestoreNetdevLabel(driver->securityManager,
+                                             vm->def, net) < 0)
+        goto cleanup;
+
+    if (virSecurityManagerTransactionCommit(driver->securityManager,
+                                            pid, priv->rememberOwner) < 0)
+        goto cleanup;
+
+    ret = 0;
+ cleanup:
+    virSecurityManagerTransactionAbort(driver->securityManager);
+    return ret;
+}
+
 
 /*
  * qemuSecurityStartVhostUserGPU:
Index: libvirt-6.0.0/src/qemu/qemu_security.h
===================================================================
--- libvirt-6.0.0.orig/src/qemu/qemu_security.h
+++ libvirt-6.0.0/src/qemu/qemu_security.h
@@ -78,6 +78,14 @@ int qemuSecurityRestoreChardevLabel(virQ
                                     virDomainObjPtr vm,
                                     virDomainChrDefPtr chr);
 
+int qemuSecuritySetNetdevLabel(virQEMUDriverPtr driver,
+                               virDomainObjPtr vm,
+                               virDomainNetDefPtr net);
+
+int qemuSecurityRestoreNetdevLabel(virQEMUDriverPtr driver,
+                                   virDomainObjPtr vm,
+                                   virDomainNetDefPtr net);
+
 int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
                                   virDomainObjPtr vm,
                                   virCommandPtr cmd,
Index: libvirt-6.0.0/src/security/security_apparmor.c
===================================================================
--- libvirt-6.0.0.orig/src/security/security_apparmor.c
+++ libvirt-6.0.0/src/security/security_apparmor.c
@@ -1065,6 +1065,64 @@ AppArmorSetSavedStateLabel(virSecurityMa
 }
 
 static int
+AppArmorSetNetdevLabel(virSecurityManagerPtr mgr,
+                       virDomainDefPtr def,
+                       virDomainNetDefPtr net)
+{
+    int ret = -1;
+    virSecurityLabelDefPtr secdef;
+    virDomainChrSourceDefPtr dev_source;
+    virDomainNetType actualType;
+
+    secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
+    if (!secdef)
+        return 0;
+
+    actualType = virDomainNetGetActualType(net);
+    if (actualType != VIR_DOMAIN_NET_TYPE_VHOSTUSER)
+        return 0;
+
+    dev_source = net->data.vhostuser;
+    switch ((virDomainChrType)dev_source->type) {
+    case VIR_DOMAIN_CHR_TYPE_UNIX:
+        ret = reload_profile(mgr, def, dev_source->data.file.path, true);
+        break;
+
+    case VIR_DOMAIN_CHR_TYPE_DEV:
+    case VIR_DOMAIN_CHR_TYPE_FILE:
+    case VIR_DOMAIN_CHR_TYPE_PTY:
+    case VIR_DOMAIN_CHR_TYPE_PIPE:
+    case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+    case VIR_DOMAIN_CHR_TYPE_NULL:
+    case VIR_DOMAIN_CHR_TYPE_VC:
+    case VIR_DOMAIN_CHR_TYPE_STDIO:
+    case VIR_DOMAIN_CHR_TYPE_UDP:
+    case VIR_DOMAIN_CHR_TYPE_TCP:
+    case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+    case VIR_DOMAIN_CHR_TYPE_NMDM:
+    case VIR_DOMAIN_CHR_TYPE_LAST:
+        ret = 0;
+        break;
+    }
+
+    return ret;
+}
+
+static int
+AppArmorRestoreNetdevLabel(virSecurityManagerPtr mgr,
+                           virDomainDefPtr def,
+                           virDomainNetDefPtr net G_GNUC_UNUSED)
+{
+    virSecurityLabelDefPtr secdef;
+
+    secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
+    if (!secdef)
+        return 0;
+
+    return reload_profile(mgr, def, NULL, false);
+}
+
+static int
 AppArmorSetPathLabel(virSecurityManagerPtr mgr,
                            virDomainDefPtr def,
                            const char *path,
@@ -1181,6 +1239,9 @@ virSecurityDriver virAppArmorSecurityDri
     .domainSetSecurityChardevLabel      = AppArmorSetChardevLabel,
     .domainRestoreSecurityChardevLabel  = AppArmorRestoreChardevLabel,
 
+    .domainSetSecurityNetdevLabel       = AppArmorSetNetdevLabel,
+    .domainRestoreSecurityNetdevLabel   = AppArmorRestoreNetdevLabel,
+
     .domainSetSecurityImageFDLabel      = AppArmorSetFDLabel,
     .domainSetSecurityTapFDLabel        = AppArmorSetFDLabel,
 
Index: libvirt-6.0.0/src/security/security_driver.h
===================================================================
--- libvirt-6.0.0.orig/src/security/security_driver.h
+++ libvirt-6.0.0/src/security/security_driver.h
@@ -152,6 +152,12 @@ typedef int (*virSecurityDomainSetTPMLab
                                               virDomainDefPtr def);
 typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManagerPtr mgr,
                                                   virDomainDefPtr def);
+typedef int (*virSecurityDomainSetNetdevLabel) (virSecurityManagerPtr mgr,
+                                                virDomainDefPtr def,
+                                                virDomainNetDefPtr net);
+typedef int (*virSecurityDomainRestoreNetdevLabel) (virSecurityManagerPtr mgr,
+                                                    virDomainDefPtr def,
+                                                    virDomainNetDefPtr net);
 
 
 struct _virSecurityDriver {
@@ -217,6 +223,9 @@ struct _virSecurityDriver {
 
     virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
     virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels;
+
+    virSecurityDomainSetNetdevLabel domainSetSecurityNetdevLabel;
+    virSecurityDomainRestoreNetdevLabel domainRestoreSecurityNetdevLabel;
 };
 
 virSecurityDriverPtr virSecurityDriverLookup(const char *name,
Index: libvirt-6.0.0/src/security/security_manager.c
===================================================================
--- libvirt-6.0.0.orig/src/security/security_manager.c
+++ libvirt-6.0.0/src/security/security_manager.c
@@ -1245,6 +1245,44 @@ virSecurityManagerRestoreTPMLabels(virSe
 }
 
 
+int
+virSecurityManagerSetNetdevLabel(virSecurityManagerPtr mgr,
+                                 virDomainDefPtr vm,
+                                 virDomainNetDefPtr net)
+{
+    int ret;
+
+    if (mgr->drv->domainSetSecurityNetdevLabel) {
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityNetdevLabel(mgr, vm, net);
+        virObjectUnlock(mgr);
+
+        return ret;
+    }
+
+    return 0;
+}
+
+
+int
+virSecurityManagerRestoreNetdevLabel(virSecurityManagerPtr mgr,
+                                     virDomainDefPtr vm,
+                                     virDomainNetDefPtr net)
+{
+    int ret;
+
+    if (mgr->drv->domainRestoreSecurityNetdevLabel) {
+        virObjectLock(mgr);
+        ret = mgr->drv->domainRestoreSecurityNetdevLabel(mgr, vm, net);
+        virObjectUnlock(mgr);
+
+        return ret;
+    }
+
+    return 0;
+}
+
+
 struct _virSecurityManagerMetadataLockState {
     size_t nfds;
     int *fds;
Index: libvirt-6.0.0/src/security/security_manager.h
===================================================================
--- libvirt-6.0.0.orig/src/security/security_manager.h
+++ libvirt-6.0.0/src/security/security_manager.h
@@ -201,6 +201,14 @@ int virSecurityManagerSetTPMLabels(virSe
 int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
                                        virDomainDefPtr vm);
 
+int virSecurityManagerSetNetdevLabel(virSecurityManagerPtr mgr,
+                                     virDomainDefPtr vm,
+                                     virDomainNetDefPtr net);
+
+int virSecurityManagerRestoreNetdevLabel(virSecurityManagerPtr mgr,
+                                         virDomainDefPtr vm,
+                                         virDomainNetDefPtr net);
+
 typedef struct _virSecurityManagerMetadataLockState virSecurityManagerMetadataLockState;
 typedef virSecurityManagerMetadataLockState *virSecurityManagerMetadataLockStatePtr;
 
Index: libvirt-6.0.0/src/security/security_stack.c
===================================================================
--- libvirt-6.0.0.orig/src/security/security_stack.c
+++ libvirt-6.0.0/src/security/security_stack.c
@@ -928,6 +928,55 @@ virSecurityStackRestoreTPMLabels(virSecu
 }
 
 
+static int
+virSecurityStackDomainSetNetdevLabel(virSecurityManagerPtr mgr,
+                                     virDomainDefPtr def,
+                                     virDomainNetDefPtr net)
+{
+    virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+    virSecurityStackItemPtr item = priv->itemsHead;
+
+    for (; item; item = item->next) {
+        if (virSecurityManagerSetNetdevLabel(item->securityManager, def, net) < 0)
+            goto rollback;
+    }
+
+    return 0;
+
+ rollback:
+    for (item = item->prev; item; item = item->prev) {
+        if (virSecurityManagerRestoreNetdevLabel(item->securityManager,
+                                                 def, net) < 0) {
+            VIR_WARN("Unable to restore netdev label after failed set label "
+                     "call virDriver=%s driver=%s domain=%s",
+                     virSecurityManagerGetVirtDriver(mgr),
+                     virSecurityManagerGetDriver(item->securityManager),
+                     def->name);
+        }
+    }
+    return -1;
+}
+
+
+static int
+virSecurityStackDomainRestoreNetdevLabel(virSecurityManagerPtr mgr,
+                                         virDomainDefPtr def,
+                                         virDomainNetDefPtr net)
+{
+    virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+    virSecurityStackItemPtr item = priv->itemsHead;
+    int rc = 0;
+
+    for (; item; item = item->next) {
+        if (virSecurityManagerRestoreNetdevLabel(item->securityManager,
+                                                 def, net) < 0)
+            rc = -1;
+    }
+
+    return rc;
+}
+
+
 virSecurityDriver virSecurityDriverStack = {
     .privateDataLen                     = sizeof(virSecurityStackData),
     .name                               = "stack",
@@ -991,4 +1040,7 @@ virSecurityDriver virSecurityDriverStack
 
     .domainSetSecurityTPMLabels         = virSecurityStackSetTPMLabels,
     .domainRestoreSecurityTPMLabels     = virSecurityStackRestoreTPMLabels,
+
+    .domainSetSecurityNetdevLabel      = virSecurityStackDomainSetNetdevLabel,
+    .domainRestoreSecurityNetdevLabel  = virSecurityStackDomainRestoreNetdevLabel,
 };

Places

File 2e8ebfe3-label-vhostuser-port.patch of Package libvirt.22291

Places