7:51 a.m.
On 6/9/25 13:59, Tim Small via Devel wrote:
From: Tim Small <tim(a)seoss.co.uk>
Add check for <forward type='hostdev'> networks which were previously
neglected (as opposed to explicit PCI hostdev devices), so that they can
be granted the necessary permissions for PCI device access. The network
type lookup in-turn requires the helper to read libvirt.conf
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993856
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
Changes since earlier patch versions:
Since V2:
. Fix missing from line in patch body
. Add this narrative
Since V1:
. Formatting - ref Peter Krempa's feedback
. Comments - ref Peter Krempa's feedback
. Minimise calls to virDomainNetResolveActualType() since it obtains
info via IPC calls - ref Peter Krempa's feedback
.../usr.lib.libvirt.virt-aa-helper.in | 4 ++++
src/security/virt-aa-helper.c | 20 +++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
index e209a8bff7..4cbad6986d 100644
--- a/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
+++ b/src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in
@@ -49,6 +49,10 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
@sysconfdir(a)/apparmor.d/libvirt/* r,
@sysconfdir@/apparmor.d/libvirt/libvirt-(a){UUID}* rw,
+ # The helper may read libvirt.conf in the course of connecting to a running
+ # libvirt deamon e.g. to resolve network configuration for a given domain
+ @sysconfdir(a)/libvirt/libvirt.conf r,
+
# for backingstore -- allow access to non-hidden files in @{HOME} as well
# as storage pools
audit deny @{HOME}/.* mrwkl,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 6481e9cfd7..f1d8feee11 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1143,6 +1143,16 @@ get_files(vahControl * ctl)
vhu->type) != 0)
goto cleanup;
}
+ /*
Nit pick - put an empty line after the previous block and before
starting this comment. Also, there's no need to start the comment with
an empty line.
+ * Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'>
+ * networks. Calling virDomainNetResolveActualType() results in IPC.
+ */
+ if (!needsVfio &&
+ net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ needsVfio = true;
+ }
}
for (i = 0; i < ctl->def->nmems; i++) {
@@ -1301,12 +1311,22 @@ get_files(vahControl * ctl)
if (ctl->def->virtType == VIR_DOMAIN_VIRT_KVM) {
for (i = 0; i < ctl->def->nnets; i++) {
virDomainNetDef *net = ctl->def->nets[i];
+
if (net && virDomainNetGetModelString(net)) {
if (net->driver.virtio.name == VIR_DOMAIN_NET_DRIVER_TYPE_QEMU)
continue;
if (!virDomainNetIsVirtioModel(net))
continue;
}
+
+ /* n.b. Calling virDomainNetResolveActualType() results in IPC. */
+ if (!needsvhost &&
+ net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ continue;
+ }
+
needsvhost = true;
}
}
But this new IPC got me thinking - the XML we're parsing here in the aa
helper is formatted by libvirtd who already know the actual type of each
net. But looking into the code - this is lost when the daemon talks to
the helper. I'm not on an AppArmor machine right now, so can you please
check whether the following change (applied on the top of this very
patch) works? It would save us from those IPCs.
diff --git i/src/security/security_apparmor.c w/src/security/security_apparmor.c
index 68ac39611f..90e4488b1f 100644
--- i/src/security/security_apparmor.c
+++ w/src/security/security_apparmor.c
@@ -156,6 +156,7 @@ load_profile(virSecurityManager *mgr G_GNUC_UNUSED,
if (virDomainDefFormatInternal(def, NULL, &buf,
VIR_DOMAIN_DEF_FORMAT_SECURE |
+ VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET |
VIR_DOMAIN_DEF_FORMAT_VOLUME_TRANSLATED) < 0)
return -1;
diff --git i/src/security/virt-aa-helper.c w/src/security/virt-aa-helper.c
index f1d8feee11..00cea448db 100644
--- i/src/security/virt-aa-helper.c
+++ w/src/security/virt-aa-helper.c
@@ -643,6 +643,7 @@ get_definition(vahControl * ctl, const char *xmlStr)
ctl->xmlopt, NULL,
VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL |
VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE |
+ VIR_DOMAIN_DEF_PARSE_ACTUAL_NET |
VIR_DOMAIN_DEF_PARSE_VOLUME_TRANSLATED);
if (ctl->def == NULL) {
@@ -1143,14 +1144,13 @@ get_files(vahControl * ctl)
vhu->type) != 0)
goto cleanup;
}
- /*
- * Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'>
+
+ /* Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'>
* networks. Calling virDomainNetResolveActualType() results in IPC.
*/
if (!needsVfio &&
net &&
- net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
- virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
needsVfio = true;
}
}
@@ -1319,11 +1319,10 @@ get_files(vahControl * ctl)
continue;
}
- /* n.b. Calling virDomainNetResolveActualType() results in IPC. */
+ /* N.B. calling virDomainNetResolveActualType() results in IPC. */
if (!needsvhost &&
net &&
- net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
- virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV){
continue;
}
Michal