7:59 a.m.
Fixes a bug whereby apparmor permissions aren't granted to allow a PCI
SR-IOV virtual function to be used in a kvm guest when the VF is defined
via a forward type='hostdev' network (as per the 'hostdev' option
documented here: https://libvirt.org/formatnetwork.html#connectivity ).
Downstream bug here:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993856
qemu accesses these PCI virtual functions using the vfio API, so no
additional permissions to access to the PCI device resources etc. via
/sys/devices/pci[...]/resource et al. are necessary.
This is a resend with fixed From in body for the patch emails, and
change notes in patch emails.
Thanks,
Tim.
Tim Small (2):
virt-aa-helper: refactor for readability
virt-aa-helper: Allow SR-IOV VF PCI for hostdev networks
.../usr.lib.libvirt.virt-aa-helper.in | 4 +++
src/security/virt-aa-helper.c | 28 ++++++++++++++++---
2 files changed, 28 insertions(+), 4 deletions(-)
--
2.47.2
7:59 a.m.
New subject: [PATCH v3 1/2] virt-aa-helper: refactor for readability
From: Tim Small <tim(a)seoss.co.uk>
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
src/security/virt-aa-helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index e3802c18be..6481e9cfd7 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1134,10 +1134,10 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nnets; i++) {
- if (ctl->def->nets[i] &&
- ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
- ctl->def->nets[i]->data.vhostuser) {
- virDomainChrSourceDef *vhu = ctl->def->nets[i]->data.vhostuser;
+ virDomainNetDef *net = ctl->def->nets[i];
+
+ if (net && net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
net->data.vhostuser) {
+ virDomainChrSourceDef *vhu = net->data.vhostuser;
if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw",
vhu->type) != 0)
--
2.47.2
8:51 a.m.
New subject: [PATCH v3 1/2] virt-aa-helper: refactor for readability
On 6/9/25 13:59, Tim Small via Devel wrote:
From: Tim Small <tim(a)seoss.co.uk>
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
src/security/virt-aa-helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index e3802c18be..6481e9cfd7 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1134,10 +1134,10 @@ get_files(vahControl * ctl)
}
for (i = 0; i < ctl->def->nnets; i++) {
- if (ctl->def->nets[i] &&
- ctl->def->nets[i]->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
&&
- ctl->def->nets[i]->data.vhostuser) {
- virDomainChrSourceDef *vhu = ctl->def->nets[i]->data.vhostuser;
+ virDomainNetDef *net = ctl->def->nets[i];
+
+ if (net && net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
net->data.vhostuser) {
Pre-existing, but I always wondered how can ctl->def->nets[i] be NULL
when we're iterating over nnets. It can't. I'll probably post a patch
that removes these non-NULL checks in the whole function.
+ virDomainChrSourceDef *vhu = net->data.vhostuser;
if (vah_add_file_chardev(&buf, vhu->data.nix.path, "rw",
vhu->type) != 0)
Michal
7:59 a.m.
New subject: [PATCH v3 2/2] virt-aa-helper: Allow SR-IOV VF PCI for hostdev networks
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;
}
+ /*
+ * 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;
}
}
--
2.47.2
8:51 a.m.
New subject: [PATCH v3 2/2] virt-aa-helper: Allow SR-IOV VF PCI for hostdev
networks
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
5
days inactive
5
days old
4 comments
2 participants
participants (2)
-
Michal Prívozník -
Tim Small