There are three internal APIs implemented in this security_util
file: virSecurityGetRememberedLabel(),
virSecuritySetRememberedLabel() and
virSecurityMoveRememberedLabel() for getting, setting and moving
remembered seclabel. All three have a special return value of -2
when XATTRs are not supported (for whatever reason) and callers
are expected to handle it gracefully. However, after my commit of
v5.7.0-rc1~115 it may happen that one of the three functions
returned -1 even though XATTRs are not supported (and thus -2
should have been returned).

Fixes: 7cfb7aab573a031880a1f4fd20747843fea109ba
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/security/security_util.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/security/security_util.c b/src/security/security_util.c
index 7fa5163fe4..622bd901ee 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -269,8 +269,11 @@ virSecurityGetRememberedLabel(const char *name,
 
     *label = NULL;
 
-    if (!(ref_name = virSecurityGetRefCountAttrName(name)))
+    if (!(ref_name = virSecurityGetRefCountAttrName(name))) {
+        if (errno == ENOSYS)
+            return -2;
         return -1;
+    }
 
     if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
         if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP)
@@ -364,8 +367,11 @@ virSecuritySetRememberedLabel(const char *name,
     g_autofree char *value = NULL;
     unsigned int refcount = 0;
 
-    if (!(ref_name = virSecurityGetRefCountAttrName(name)))
+    if (!(ref_name = virSecurityGetRefCountAttrName(name))) {
+        if (errno == ENOSYS)
+            return -2;
         return -1;
+    }
 
     if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
         if (errno == ENOSYS || errno == ENOTSUP) {
@@ -452,8 +458,11 @@ virSecurityMoveRememberedLabel(const char *name,
 
     if (!(ref_name = virSecurityGetRefCountAttrName(name)) ||
         !(attr_name = virSecurityGetAttrName(name)) ||
-        !(timestamp_name = virSecurityGetTimestampAttrName(name)))
+        !(timestamp_name = virSecurityGetTimestampAttrName(name))) {
+        if (errno == ENOSYS)
+            return -2;
         return -1;
+    }
 
     if (virFileGetXAttrQuiet(src, ref_name, &ref_value) < 0) {
         if (errno == ENOSYS || errno == ENOTSUP) {
-- 
2.26.2

On Tue, Nov 03, 2020 at 02:13:26PM +0100, Michal Privoznik wrote:
> There are three internal APIs implemented in this security_util
> file: virSecurityGetRememberedLabel(),
> virSecuritySetRememberedLabel() and
> virSecurityMoveRememberedLabel() for getting, setting and moving
> remembered seclabel. All three have a special return value of -2
> when XATTRs are not supported (for whatever reason) and callers
> are expected to handle it gracefully. However, after my commit of
> v5.7.0-rc1~115 it may happen that one of the three functions
> returned -1 even though XATTRs are not supported (and thus -2
> should have been returned).
> 
> Fixes: 7cfb7aab573a031880a1f4fd20747843fea109ba
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/security/security_util.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/src/security/security_util.c b/src/security/security_util.c
> index 7fa5163fe4..622bd901ee 100644
> --- a/src/security/security_util.c
> +++ b/src/security/security_util.c
> @@ -269,8 +269,11 @@ virSecurityGetRememberedLabel(const char *name,
>  
>      *label = NULL;
>  
> -    if (!(ref_name = virSecurityGetRefCountAttrName(name)))
> +    if (!(ref_name = virSecurityGetRefCountAttrName(name))) {
> +        if (errno == ENOSYS)
> +            return -2;
>          return -1;
> +    }
>  
>      if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
>          if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP)
> @@ -364,8 +367,11 @@ virSecuritySetRememberedLabel(const char *name,
>      g_autofree char *value = NULL;
>      unsigned int refcount = 0;
>  
> -    if (!(ref_name = virSecurityGetRefCountAttrName(name)))
> +    if (!(ref_name = virSecurityGetRefCountAttrName(name))) {
> +        if (errno == ENOSYS)
> +            return -2;
>          return -1;
> +    }
>  
>      if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
>          if (errno == ENOSYS || errno == ENOTSUP) {
> @@ -452,8 +458,11 @@ virSecurityMoveRememberedLabel(const char *name,
>  
>      if (!(ref_name = virSecurityGetRefCountAttrName(name)) ||
>          !(attr_name = virSecurityGetAttrName(name)) ||
> -        !(timestamp_name = virSecurityGetTimestampAttrName(name)))
> +        !(timestamp_name = virSecurityGetTimestampAttrName(name))) {
> +        if (errno == ENOSYS)
> +            return -2;
>          return -1;
> +    }
>  
>      if (virFileGetXAttrQuiet(src, ref_name, &ref_value) < 0) {
>          if (errno == ENOSYS || errno == ENOTSUP) {
> -- 
> 2.26.2
> 

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>

It might be a separate issue, but I wonder if we can report a warning
instead of system error if protected xattrs aren't supported to avoid
confusion:

  static char *
  virSecurityGetAttrName(const char *name G_GNUC_UNUSED)
  {
      char *ret = NULL;
  #ifdef XATTR_NAMESPACE
      ret = g_strdup_printf(XATTR_NAMESPACE".libvirt.security.%s", name);
  #else
      errno = ENOSYS;
      virReportSystemError(errno, "%s",
                           _("Extended attributes are not supported on this system"));
  #endif
      return ret;
  }

Thanks,
Roman