Re: [PATCH RESEND 09/20] dac, selinux: skip setting/restoring label for absent PCI devices
On 1/18/21 2:53 PM, Daniel Henrique Barboza wrote:
If the underlying PCI device of a hostdev does not exist in the
host (e.g. a SR-IOV VF that was removed while the domain was
running), skip security label handling for it.
This will avoid errors that happens during qemuProcessStop() time,
where a VF that was being used by the domain is not present anymore.
The restore label functions of both DAC and SELinux drivers will
trigger errors in virPCIDeviceNew().
Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com>
This is fine as far as it goes, but what about
AppArmorSetSecurityHostdevLabel() (and also whatever it is that's going
on in the get_files() function in virt-aa-helper.c). You likely don't
have a setup to test it, but it seems pretty straightforward to
extrapolate that if you should skip setting the selinux and DAC labels
when a device doesn't exist, you can probably do the same thing for
AppArmor.
Reviewed-by: Laine Stump <laine(a)redhat.com>
but add another patch that fixes the problem for AppArmor too.
(Also, all the code repetition here makes me think that there must be a
better way to do this and reduce all the boilerplate, but I think it
would be better to just make these changes and then look at eliminating
the boilerplate afterwards, rather than re-structuring the code (which
could create regressions) while adding this new concept on top of it.
---
src/security/security_dac.c | 14 ++++++++++++--
src/security/security_selinux.c | 14 ++++++++++++--
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0085982bb1..a2528aeb2d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1266,7 +1266,12 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
+ g_autoptr(virPCIDevice) pci = NULL;
+
+ if (!virPCIDeviceExists(&pcisrc->addr))
+ break;
+
+ pci = virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;
@@ -1422,7 +1427,12 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
+ g_autoptr(virPCIDevice) pci = NULL;
+
+ if (!virPCIDeviceExists(&pcisrc->addr))
+ break;
+
+ pci = virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 99adf08a15..c018c0708a 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2101,7 +2101,12 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManagerPtr
mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
+ g_autoptr(virPCIDevice) pci = NULL;
+
+ if (!virPCIDeviceExists(&pcisrc->addr))
+ break;
+
+ pci = virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;
@@ -2329,7 +2334,12 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManagerPtr
mgr,
}
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: {
- g_autoptr(virPCIDevice) pci = virPCIDeviceNew(&pcisrc->addr);
+ g_autoptr(virPCIDevice) pci = NULL;
+
+ if (!virPCIDeviceExists(&pcisrc->addr))
+ break;
+
+ pci = virPCIDeviceNew(&pcisrc->addr);
if (!pci)
return -1;