The point of virSecurityManagerRestoreAllLabel() function is to
restore ALL labels and be tolerant to possible errors, i.e.
continue restoring seclabels and NOT return early.

Well, in two implementations of this internal API this type of
problem was found:

1) virSecurityDACRestoreAllLabel() returned early if
   virSecurityDACRestoreGraphicsLabel() failed, or when
   def->sec->sectype equals to an impossible value.

2) virSecuritySELinuxRestoreAllLabel() returned early if
   virSecuritySELinuxRestoreMemoryLabel() failed.

Fix all three places.

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

Inspired by this Peter's patch:

https://marc.info/?l=libvir-list&m=174169408218982&w=2

 src/security/security_dac.c     | 4 ++--
 src/security/security_selinux.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index e07977300f..3ecbc7277d 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -1973,7 +1973,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr,
 
     for (i = 0; i < def->ngraphics; i++) {
         if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0)
-            return -1;
+            rc = -1;
     }
 
     for (i = 0; i < def->ninputs; i++) {
@@ -2021,7 +2021,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr,
         case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
         case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
             virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype);
-            return -1;
+            rc = -1;
         }
     }
 
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 38e611f567..64e7f41ce0 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -2969,7 +2969,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr,
 
     for (i = 0; i < def->nmems; i++) {
         if (virSecuritySELinuxRestoreMemoryLabel(mgr, def, def->mems[i]) < 0)
-            return -1;
+            rc = -1;
     }
 
     for (i = 0; i < def->ntpms; i++) {
-- 
2.48.1

On Tue, Mar 11, 2025 at 13:56:44 +0100, Michal Privoznik wrote:
> The point of virSecurityManagerRestoreAllLabel() function is to
> restore ALL labels and be tolerant to possible errors, i.e.
> continue restoring seclabels and NOT return early.
> 
> Well, in two implementations of this internal API this type of
> problem was found:
> 
> 1) virSecurityDACRestoreAllLabel() returned early if
>    virSecurityDACRestoreGraphicsLabel() failed, or when
>    def->sec->sectype equals to an impossible value.
> 
> 2) virSecuritySELinuxRestoreAllLabel() returned early if
>    virSecuritySELinuxRestoreMemoryLabel() failed.
> 
> Fix all three places.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> Inspired by this Peter's patch:
> 
> https://marc.info/?l=libvir-list&m=174169408218982&w=2
> 
>  src/security/security_dac.c     | 4 ++--
>  src/security/security_selinux.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index e07977300f..3ecbc7277d 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -1973,7 +1973,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr,
>  
>      for (i = 0; i < def->ngraphics; i++) {
>          if (virSecurityDACRestoreGraphicsLabel(mgr, def, def->graphics[i]) < 0)
> -            return -1;
> +            rc = -1;
>      }
>  
>      for (i = 0; i < def->ninputs; i++) {
> @@ -2021,7 +2021,7 @@ virSecurityDACRestoreAllLabel(virSecurityManager *mgr,
>          case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
>          case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
>              virReportEnumRangeError(virDomainLaunchSecurity, def->sec->sectype);
> -            return -1;
> +            rc = -1;
>          }
>      }
>  
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 38e611f567..64e7f41ce0 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -2969,7 +2969,7 @@ virSecuritySELinuxRestoreAllLabel(virSecurityManager *mgr,
>  
>      for (i = 0; i < def->nmems; i++) {
>          if (virSecuritySELinuxRestoreMemoryLabel(mgr, def, def->mems[i]) < 0)
> -            return -1;
> +            rc = -1;
>      }
>  
>      for (i = 0; i < def->ntpms; i++) {
> -- 
> 2.48.1
> 

Reviewed-by: Peter Krempa <pkrempa@redhat.com>