11 a.m.
I'm working on a fix for 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
I'm not sure if the attached patches are sufficient. When I compare the
apparmor permissions file for a guest with a VF shared via forward
type='hostdev' vs. the same guest with a VF shared via a PCI hostdev
device, the latter has extra apparmor permissions for files like:
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/resource3_wc" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/resource0_wc" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/resource3" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/vendor" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/reset" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/resource" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/device" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/resource0" rw,
"/sys/devices/pci0000:00/0000:00:1d.0/0000:03:10.0/config" rw,
... however both guests appear to function in my test environment
(Debian 13, 6.12.22-amd64) - i.e. both with and without those entries.
So I don't know if those permissions are unneeded, or if they are
granted at runtime instead. If those permissions are needed, then I'd
appreciate any hints on how to modify virt-aa-helper to discover the PCI
address. I appreciate that might not actually be possible because that
is dynamically allocated, and so might race - so some other solution
might be required.
Many Thanks,
Tim.
Tim Small (2):
virt-aa-helper: refactor for readability
virt-aa-helper: Allow SR-IOV VF PCI for hostdev networks
.../apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 +++
src/security/virt-aa-helper.c | 18 ++++++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
--
2.47.2
11 a.m.
New subject: [RFC PATCH 1/2] virt-aa-helper: refactor for readability
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
src/security/virt-aa-helper.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index e3802c18be..fa69245324 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1134,10 +1134,9 @@ 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
2:50 a.m.
New subject: [RFC PATCH 1/2] virt-aa-helper: refactor for readability
On Tue, May 06, 2025 at 17:00:10 +0100, Tim Small via Devel wrote:
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
src/security/virt-aa-helper.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Note my comment about git config for authorship on 2/2;
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
11 a.m.
New subject: [RFC PATCH 2/2] virt-aa-helper: Allow SR-IOV VF PCI for hostdev networks
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
Downstream bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993856
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
.../apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 +++
src/security/virt-aa-helper.c | 11 +++++++++++
2 files changed, 14 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..3b3d733b5e 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,9 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
@sysconfdir(a)/apparmor.d/libvirt/* r,
@sysconfdir@/apparmor.d/libvirt/libvirt-(a){UUID}* rw,
+ # allow network type lookup to check for forward type=hostdev networks
+ @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 fa69245324..7228292358 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1142,6 +1142,12 @@ get_files(vahControl * ctl)
vhu->type) != 0)
goto cleanup;
}
+ /* Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'> networks */
+ if (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++) {
@@ -1306,6 +1312,11 @@ get_files(vahControl * ctl)
if (!virDomainNetIsVirtioModel(net))
continue;
}
+ if (net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ continue;
+ }
needsvhost = true;
}
}
--
2.47.2
2:47 a.m.
New subject: [RFC PATCH 2/2] virt-aa-helper: Allow SR-IOV VF PCI for hostdev
networks
On Tue, May 06, 2025 at 17:00:11 +0100, Tim Small via Devel wrote:
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
Downstream bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=993856
Signed-off-by: Tim Small <tim(a)seoss.co.uk>
---
Your email domain uses DMARC so our mailing list applied
countermeasures. This means that your authorship of the patch would be
wrong. Please configure git to always add the extra 'From:' field:
https://www.libvirt.org/submitting-patches.html#git-configuration
Notably:
$ git config --global format.forceInBodyFrom true
.../apparmor/usr.lib.libvirt.virt-aa-helper.in | 3 +++
src/security/virt-aa-helper.c | 11 +++++++++++
2 files changed, 14 insertions(+)
Also note that I'm no apparmor expert; I'm merely mentioning what I've
noticed.
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..3b3d733b5e 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,9 @@ profile virt-aa-helper @libexecdir@/virt-aa-helper {
@sysconfdir(a)/apparmor.d/libvirt/* r,
@sysconfdir@/apparmor.d/libvirt/libvirt-(a){UUID}* rw,
+ # allow network type lookup to check for forward type=hostdev networks
I presume this is needed to open outbound connections to the appropriate
sub-daemon; so not specific to hostdev lookup. Preferrably reword it so
that it states that the helper might be needing client connections.
+ @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 fa69245324..7228292358 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1142,6 +1142,12 @@ get_files(vahControl * ctl)
vhu->type) != 0)
goto cleanup;
}
+ /* Grant vfio for SR-IOV PCI VFs shared via <forward
type='hostdev'> networks */
+ if (net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
The indentation is broken here.
As 'virDomainNetResolveActualType' opens connection and does a pretty
expensive lookup I'd suggest trying to check if given network is a
hostdev only when 'needsVfio' is still false.
Unfortunately this will need to be done for all the cases when this is
a non-hostdev ...
+ needsVfio = true;
+ }
}
for (i = 0; i < ctl->def->nmems; i++) {
@@ -1306,6 +1312,11 @@ get_files(vahControl * ctl)
if (!virDomainNetIsVirtioModel(net))
continue;
}
+ if (net &&
+ net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
here too
+ continue;
IIUC this shouldn't be needed. By definition hostdev-based networks
can't be 'virito' so they should have been already skipped by the above
condition.
+ }
needsvhost = true;
}
}
--
2.47.2
12
days inactive
13
days old
4 comments
2 participants
participants (2)
-
Peter Krempa -
Tim Small