[PATCH RESEND 09/20] dac, selinux: skip setting/restoring label for absent PCI devices
Laine Stump
laine at redhat.com
Mon Feb 22 03:39:33 UTC 2021
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 at 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 at 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;
>
More information about the libvir-list
mailing list