1. Patchew
  2. Libvirt
  3. View series

[PATCH] security: do not remember/recall labels for VFIO MDEVs

Eric Farman posted 1 patch 2 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230401004204.496246-1-farman@linux.ibm.com
src/security/security_dac.c     | 4 ++--
src/security/security_selinux.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
[PATCH] security: do not remember/recall labels for VFIO MDEVs
Posted by Eric Farman 2 years, 10 months ago 
Commit dbf1f68410 ("security: do not remember/recall labels for VFIO")
rightly changed the DAC and SELinux labeling parameters to fix a problem
with "VFIO hostdevs" but really only addressed the PCI codepaths.
As a result, we can still encounter this with VFIO MDEVs such as
vfio-ccw and vfio-ap, which can fail on a hotplug:

  [test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-eff72715c4f0
  [test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-eff72715c4f0
  [test@host ~]# cat disk.xml
    <hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
      <source>
        <address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/>
      </source>
      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/>
    </hostdev>
  [test@host ~]# virsh attach-device guest ~/disk.xml
  error: Failed to attach device from /home/test/disk.xml
  error: Requested operation is not valid: Setting different SELinux label on /dev/vfio/3 which is already in use

Make the same changes as reported in commit dbf1f68410, for the mdev paths.

Reported-by: Matthew Rosato <mjrosato@linux.ibm.com>
Signed-off-by: Eric Farman <farman@linux.ibm.com>
---
 src/security/security_dac.c     | 4 ++--
 src/security/security_selinux.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 9be8f458d1..bceb6a5c24 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1310,7 +1310,7 @@ virSecurityDACSetHostdevLabel(virSecurityManager *mgr,
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return -1;
 
-        ret = virSecurityDACSetHostdevLabelHelper(vfiodev, true, &cbdata);
+        ret = virSecurityDACSetHostdevLabelHelper(vfiodev, false, &cbdata);
         break;
     }
 
@@ -1466,7 +1466,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManager *mgr,
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return -1;
 
-        ret = virSecurityDACRestoreFileLabel(mgr, vfiodev);
+        ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL, vfiodev, false);
         break;
     }
 
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index e43962435f..9c23735aa3 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2217,7 +2217,7 @@ virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManager *mgr,
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return ret;
 
-        ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, true, &data);
+        ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, false, &data);
         break;
     }
 
@@ -2445,7 +2445,7 @@ virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManager *mgr,
         if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc->uuidstr)))
             return -1;
 
-        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, true);
+        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev, false);
         break;
     }
 
-- 
2.37.2
Re: [PATCH] security: do not remember/recall labels for VFIO MDEVs
Posted by Michal Prívozník 2 years, 10 months ago 
On 4/1/23 02:42, Eric Farman wrote:
> Commit dbf1f68410 ("security: do not remember/recall labels for VFIO")
> rightly changed the DAC and SELinux labeling parameters to fix a problem
> with "VFIO hostdevs" but really only addressed the PCI codepaths.
> As a result, we can still encounter this with VFIO MDEVs such as
> vfio-ccw and vfio-ap, which can fail on a hotplug:
> 
>   [test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-eff72715c4f0
>   [test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-eff72715c4f0
>   [test@host ~]# cat disk.xml
>     <hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
>       <source>
>         <address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/>
>       </source>
>       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/>
>     </hostdev>
>   [test@host ~]# virsh attach-device guest ~/disk.xml
>   error: Failed to attach device from /home/test/disk.xml
>   error: Requested operation is not valid: Setting different SELinux label on /dev/vfio/3 which is already in use
> 
> Make the same changes as reported in commit dbf1f68410, for the mdev paths.
> 
> Reported-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>  src/security/security_dac.c     | 4 ++--
>  src/security/security_selinux.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Oops, sorry for the delay. I marked for review when I saw this patch,
but then got side tracked and forgot about it.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and pushed.

Michal
Re: [PATCH] security: do not remember/recall labels for VFIO MDEVs
Posted by Eric Farman 2 years, 10 months ago 
On Thu, 2023-04-13 at 16:35 +0200, Michal Prívozník wrote:
> On 4/1/23 02:42, Eric Farman wrote:
> > Commit dbf1f68410 ("security: do not remember/recall labels for
> > VFIO")
> > rightly changed the DAC and SELinux labeling parameters to fix a
> > problem
> > with "VFIO hostdevs" but really only addressed the PCI codepaths.
> > As a result, we can still encounter this with VFIO MDEVs such as
> > vfio-ccw and vfio-ap, which can fail on a hotplug:
> > 
> >   [test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-
> > eff72715c4f0
> >   [test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-
> > eff72715c4f0
> >   [test@host ~]# cat disk.xml
> >     <hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
> >       <source>
> >         <address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/>
> >       </source>
> >       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/>
> >     </hostdev>
> >   [test@host ~]# virsh attach-device guest ~/disk.xml
> >   error: Failed to attach device from /home/test/disk.xml
> >   error: Requested operation is not valid: Setting different
> > SELinux label on /dev/vfio/3 which is already in use
> > 
> > Make the same changes as reported in commit dbf1f68410, for the
> > mdev paths.
> > 
> > Reported-by: Matthew Rosato <mjrosato@linux.ibm.com>
> > Signed-off-by: Eric Farman <farman@linux.ibm.com>
> > ---
> >  src/security/security_dac.c     | 4 ++--
> >  src/security/security_selinux.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> Oops, sorry for the delay. I marked for review when I saw this patch,
> but then got side tracked and forgot about it.

Not a problem; thank you for the review/push!

Eric

> 
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
> 
> and pushed.
> 
> Michal
>
Re: [PATCH] security: do not remember/recall labels for VFIO MDEVs
Posted by Eric Farman 2 years, 10 months ago 
On Sat, 2023-04-01 at 02:42 +0200, Eric Farman wrote:
> Commit dbf1f68410 ("security: do not remember/recall labels for
> VFIO")
> rightly changed the DAC and SELinux labeling parameters to fix a
> problem
> with "VFIO hostdevs" but really only addressed the PCI codepaths.
> As a result, we can still encounter this with VFIO MDEVs such as
> vfio-ccw and vfio-ap, which can fail on a hotplug:
> 
>   [test@host ~]# mdevctl stop -u 11f2d2bc-4083-431d-a023-eff72715c4f0
>   [test@host ~]# mdevctl start -u 11f2d2bc-4083-431d-a023-
> eff72715c4f0
>   [test@host ~]# cat disk.xml
>     <hostdev mode='subsystem' type='mdev' model='vfio-ccw'>
>       <source>
>         <address uuid='11f2d2bc-4083-431d-a023-eff72715c4f0'/>
>       </source>
>       <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x3c51'/>
>     </hostdev>
>   [test@host ~]# virsh attach-device guest ~/disk.xml
>   error: Failed to attach device from /home/test/disk.xml
>   error: Requested operation is not valid: Setting different SELinux
> label on /dev/vfio/3 which is already in use
> 
> Make the same changes as reported in commit dbf1f68410, for the mdev
> paths.
> 
> Reported-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Eric Farman <farman@linux.ibm.com>

Ping? I'm hoping this just got lost between the release and the
holiday, but if there's some changes needed here I'm happy to work
through that. Thank you!

> ---
>  src/security/security_dac.c     | 4 ++--
>  src/security/security_selinux.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/src/security/security_dac.c
> b/src/security/security_dac.c
> index 9be8f458d1..bceb6a5c24 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1310,7 +1310,7 @@
> virSecurityDACSetHostdevLabel(virSecurityManager *mgr,
>          if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc-
> >uuidstr)))
>              return -1;
>  
> -        ret = virSecurityDACSetHostdevLabelHelper(vfiodev, true,
> &cbdata);
> +        ret = virSecurityDACSetHostdevLabelHelper(vfiodev, false,
> &cbdata);
>          break;
>      }
>  
> @@ -1466,7 +1466,7 @@
> virSecurityDACRestoreHostdevLabel(virSecurityManager *mgr,
>          if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc-
> >uuidstr)))
>              return -1;
>  
> -        ret = virSecurityDACRestoreFileLabel(mgr, vfiodev);
> +        ret = virSecurityDACRestoreFileLabelInternal(mgr, NULL,
> vfiodev, false);
>          break;
>      }
>  
> diff --git a/src/security/security_selinux.c
> b/src/security/security_selinux.c
> index e43962435f..9c23735aa3 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2217,7 +2217,7 @@
> virSecuritySELinuxSetHostdevSubsysLabel(virSecurityManager *mgr,
>          if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc-
> >uuidstr)))
>              return ret;
>  
> -        ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev, true,
> &data);
> +        ret = virSecuritySELinuxSetHostdevLabelHelper(vfiodev,
> false, &data);
>          break;
>      }
>  
> @@ -2445,7 +2445,7 @@
> virSecuritySELinuxRestoreHostdevSubsysLabel(virSecurityManager *mgr,
>          if (!(vfiodev = virMediatedDeviceGetIOMMUGroupDev(mdevsrc-
> >uuidstr)))
>              return -1;
>  
> -        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev,
> true);
> +        ret = virSecuritySELinuxRestoreFileLabel(mgr, vfiodev,
> false);
>          break;
>      }
>  

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.