Introduce a read-only `tapfd` element for interfaces of type network,
bridge, direct, and ethernet, which contains the path in `/dev` to the
backing tapfd for that interface. The element is only included when the
domain is being formatted for internal consumption
(VIR_DOMAIN_DEF_FORMAT_STATUS) and is not accepted in user-provided XML
(!VIR_DOMAIN_DEF_PARSE_INACTIVE).
This is used by the AppArmor security driver when re-generating profiles.
Partial-Resolves: #692
Bug-Ubuntu: https://bugs.launchpad.net/bugs/2126574
Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
---
src/conf/domain_conf.c | 10 ++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 9 +++++++++
src/security/security_apparmor.c | 1 +
src/security/virt-aa-helper.c | 5 +++++
5 files changed, 26 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 541dad5bdc..bfeed3dc96 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2936,6 +2936,9 @@ virDomainNetDefFree(virDomainNetDef *def)
g_free(def->virtio);
g_free(def->coalesce);
g_free(def->sourceDev);
+ if (def->tapfdpath) {
+ g_free(def->tapfdpath);
+ }
virNetDevIPInfoClear(&def->guestIP);
virNetDevIPInfoClear(&def->hostIP);
@@ -10427,6 +10430,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
return NULL;
}
+ if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
+ def->tapfdpath = virXPathString("string(./tapfd/@path)", ctxt);
+ }
+
if (virNetworkPortOptionsParseXML(ctxt, &def->isolatedPort) < 0)
return NULL;
@@ -25596,6 +25603,9 @@ virDomainNetDefFormat(virBuffer *buf,
if (def->mtu)
virBufferAsprintf(buf, "<mtu size='%u'/>\n", def->mtu);
+ if (def->tapfdpath && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS))
+ virBufferAsprintf(buf, "<tapfd path='%s'/>\n", def->tapfdpath);
+
virDomainNetDefCoalesceFormatXML(buf, def->coalesce);
virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cb35ff06bd..cadbc3e36d 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1201,6 +1201,7 @@ struct _virDomainNetDef {
char *downscript;
char *domain_name; /* backend domain name */
char *ifname; /* interface name on the host (<target dev='x'/>) */
+ char *tapfdpath; /* Path for the device's tapfd in /dev on the host */
virTristateBool managed_tap;
virNetDevIPInfo hostIP;
char *ifname_guest_actual;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 98229d7cf9..96352821d7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8880,9 +8880,18 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
for (i = 0; i < tapfdSize; i++) {
g_autofree char *name = g_strdup_printf("tapfd-%s%zu", net->info.alias, i);
+ g_autofree char *procpath = NULL;
int fd = tapfd[i]; /* we want to keep the array intact for security labeling*/
netpriv->tapfds = g_slist_prepend(netpriv->tapfds, qemuFDPassDirectNew(name, &fd));
+
+ /* Include tapfd path in the domstatus XML */
+ procpath = g_strdup_printf("/proc/self/fd/%d", tapfd[i]);
+
+ if (virFileResolveLink(procpath, &net->tapfdpath) < 0) {
+ /* it's a deleted file, presumably. Ignore? */
+ VIR_WARN("could not find path for descriptor %s, skipping", procpath);
+ }
}
netpriv->tapfds = g_slist_reverse(netpriv->tapfds);
diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index 68ac39611f..13dbce67c3 100644
--- a/src/security/security_apparmor.c
+++ b/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_STATUS |
VIR_DOMAIN_DEF_FORMAT_VOLUME_TRANSLATED) < 0)
return -1;
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 9598b95432..7920162cdc 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -1174,6 +1174,11 @@ get_files(vahControl * ctl)
vhu->type) != 0)
return -1;
}
+
+ if (net->tapfdpath) {
+ if (vah_add_file(&buf, net->tapfdpath, "rwk") != 0)
+ return -1;
+ }
}
for (i = 0; i < ctl->def->nmems; i++) {
--
2.51.0On Mon, Jan 05, 2026 at 14:27:22 -0600, Wesley Hershberger via Devel wrote:
> Introduce a read-only `tapfd` element for interfaces of type network,
> bridge, direct, and ethernet, which contains the path in `/dev` to the
> backing tapfd for that interface. The element is only included when the
> domain is being formatted for internal consumption
> (VIR_DOMAIN_DEF_FORMAT_STATUS) and is not accepted in user-provided XML
> (!VIR_DOMAIN_DEF_PARSE_INACTIVE).
>
> This is used by the AppArmor security driver when re-generating profiles.
>
> Partial-Resolves: #692
> Bug-Ubuntu: https://bugs.launchpad.net/bugs/2126574
> Signed-off-by: Wesley Hershberger <wesley.hershberger@canonical.com>
> ---
> src/conf/domain_conf.c | 10 ++++++++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 9 +++++++++
> src/security/security_apparmor.c | 1 +
> src/security/virt-aa-helper.c | 5 +++++
> 5 files changed, 26 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 541dad5bdc..bfeed3dc96 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2936,6 +2936,9 @@ virDomainNetDefFree(virDomainNetDef *def)
> g_free(def->virtio);
> g_free(def->coalesce);
> g_free(def->sourceDev);
> + if (def->tapfdpath) {
> + g_free(def->tapfdpath);
> + }
'g_free' is a no-op if argument is NULL so no check is needed similarly
to all other calls above.
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 98229d7cf9..96352821d7 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8880,9 +8880,18 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
>
> for (i = 0; i < tapfdSize; i++) {
> g_autofree char *name = g_strdup_printf("tapfd-%s%zu", net->info.alias, i);
> + g_autofree char *procpath = NULL;
> int fd = tapfd[i]; /* we want to keep the array intact for security labeling*/
>
> netpriv->tapfds = g_slist_prepend(netpriv->tapfds, qemuFDPassDirectNew(name, &fd));
> +
> + /* Include tapfd path in the domstatus XML */
> + procpath = g_strdup_printf("/proc/self/fd/%d", tapfd[i]);
So this resolves to /proc/PID_OF_VIRTQEMUD_PROCESS/fd/NNN. virtqemud
will close the FD as soon as we pass it to qemu and additionally
virtqemud might restart where the path stored in the status XML (which
is mainly used to re-load after restart) would point to the old PID.
Thus this makes no sense to be stored in status XML.
You might want to store the path once it's passed to qemu inside the
qemu processe's fd list, but I'd argue that qemu ought to have access
to everything in there anyways since libvirt actively passed it there.
> +
> + if (virFileResolveLink(procpath, &net->tapfdpath) < 0) {
> + /* it's a deleted file, presumably. Ignore? */
'deleted file' ? These werent files originally even when they come from
e.g. opening /dev/net/tun.
> + VIR_WARN("could not find path for descriptor %s, skipping", procpath);
Please avoid VIR_WARN.
> + }
> }
>
> netpriv->tapfds = g_slist_reverse(netpriv->tapfds);
So with other security drivers the one-time setup of seclabels passed to
qemu is done in qemuSecuritySetTapFDLabel() in this function. Other
dirvers can do it one-time because it applies on the FDs passed to qemu
and doesn't need to be changed.
If apparmor requires to be able to reference the FD paths at any later
time it will need to refer to the FDs under the qemu pid (which is not
know at this point since the qemu process did not start) rather than the
pid of the process the qemu driver runs in because that one can re-start
and also will necessarily close the FDs once qemu starts.
On Thu, Jan 15, 2026 at 3:50 AM Peter Krempa <pkrempa@redhat.com> wrote:
Thanks very much for the review!
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 98229d7cf9..96352821d7 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -8880,9 +8880,18 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
> >
> > for (i = 0; i < tapfdSize; i++) {
> > g_autofree char *name = g_strdup_printf("tapfd-%s%zu", net->info.alias, i);
> > + g_autofree char *procpath = NULL;
> > int fd = tapfd[i]; /* we want to keep the array intact for security labeling*/
> >
> > netpriv->tapfds = g_slist_prepend(netpriv->tapfds, qemuFDPassDirectNew(name, &fd));
> > +
> > + /* Include tapfd path in the domstatus XML */
> > + procpath = g_strdup_printf("/proc/self/fd/%d", tapfd[i]);
>
> So this resolves to /proc/PID_OF_VIRTQEMUD_PROCESS/fd/NNN. virtqemud
> will close the FD as soon as we pass it to qemu and additionally
> virtqemud might restart where the path stored in the status XML (which
> is mainly used to re-load after restart) would point to the old PID.
Maybe I misunderstood, but this code is almost an exact copy from
AppArmorSetFDLabel,
where it resolves to "/dev/tapXX" (virFileResolveLink ends up in
g_canonicalize_path,
which must call realpath(3)). XX in that path _should_ be the index
from ioctl SIOCGIFINDEX
which IIUC isn't specific to the virtqemud process.
All that said, it's super gross to use virFileResolveLink here,
especially because
virNetDevMacVLanTapOpen already has the tapname. I'm wondering if it's
feasible to just
pass the tapname through from qemuInterfaceDirectConnect and use
qemuSecuritySetPathLabel instead of qemuSecuritySetFDLabel for macvtap devices.
I have basically no experience with SELinux; to your knowledge would
this work for that
driver as I've described it?
> Thus this makes no sense to be stored in status XML.
Would the above change your position on this?
Thanks!
~Wesley
On Mon, Jan 19, 2026 at 11:38:05 -0600, Wesley Hershberger wrote:
> On Thu, Jan 15, 2026 at 3:50 AM Peter Krempa <pkrempa@redhat.com> wrote:
>
> Thanks very much for the review!
>
> > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > > index 98229d7cf9..96352821d7 100644
> > > --- a/src/qemu/qemu_command.c
> > > +++ b/src/qemu/qemu_command.c
> > > @@ -8880,9 +8880,18 @@ qemuBuildInterfaceConnect(virDomainObj *vm,
> > >
> > > for (i = 0; i < tapfdSize; i++) {
> > > g_autofree char *name = g_strdup_printf("tapfd-%s%zu", net->info.alias, i);
> > > + g_autofree char *procpath = NULL;
> > > int fd = tapfd[i]; /* we want to keep the array intact for security labeling*/
> > >
> > > netpriv->tapfds = g_slist_prepend(netpriv->tapfds, qemuFDPassDirectNew(name, &fd));
> > > +
> > > + /* Include tapfd path in the domstatus XML */
> > > + procpath = g_strdup_printf("/proc/self/fd/%d", tapfd[i]);
> >
> > So this resolves to /proc/PID_OF_VIRTQEMUD_PROCESS/fd/NNN. virtqemud
> > will close the FD as soon as we pass it to qemu and additionally
> > virtqemud might restart where the path stored in the status XML (which
> > is mainly used to re-load after restart) would point to the old PID.
>
> Maybe I misunderstood, but this code is almost an exact copy from
> AppArmorSetFDLabel,
> where it resolves to "/dev/tapXX" (virFileResolveLink ends up in
> g_canonicalize_path,
> which must call realpath(3)). XX in that path _should_ be the index
> from ioctl SIOCGIFINDEX
> which IIUC isn't specific to the virtqemud process.
Ah so if this resolves to /dev/tap??? then that information may make
sense to be stored. But it definitely is not obvious what happens in the
code above so you'll need to comment it and explain what happens.
> All that said, it's super gross to use virFileResolveLink here,
> especially because
> virNetDevMacVLanTapOpen already has the tapname. I'm wondering if it's
> feasible to just
> pass the tapname through from qemuInterfaceDirectConnect and use
Feel free to plumb it through; qemuInterfaceDirectConnect has only one
place where it's called so that should be easy.
> qemuSecuritySetPathLabel instead of qemuSecuritySetFDLabel for macvtap devices.
> I have basically no experience with SELinux; to your knowledge would
> this work for that
> driver as I've described it?
AFAIU you need to label the specific FD here for selinux to work
properly. Since it's being passed to qemu we still need the FD.
You could introduce an API which takes both FD and path to label and
apparmor would label the latter, but you can't avoid the FD wit selinux.
© 2016 - 2026 Red Hat, Inc.