[libvirt] [PATCH 1/1] security_util: verify xattrs only if ref is present

[Michal Privoznik]

On 8/28/19 12:21 PM, Nikolay Shirokovskiy wrote:
> After 7cfb7aab573 commit starting a domain pullutes logs with
> warnings like [1]. The reason is resource files do not
> have timestamp before starting a domain and after destroying
> domain the timestamp is cleared. Let's check the timestamp
> only if attribute with refcounter is found.
> 
> [1] warning : virSecurityValidateTimestamp:198 : Invalid XATTR timestamp detected on \
>      /some/path secdriver=dac
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>   src/security/security_util.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/src/security/security_util.c b/src/security/security_util.c
> index 31f41cedfd..f33fe9dd7b 100644
> --- a/src/security/security_util.c
> +++ b/src/security/security_util.c
> @@ -269,13 +269,9 @@ virSecurityGetRememberedLabel(const char *name,
>       VIR_AUTOFREE(char *) attr_name = NULL;
>       VIR_AUTOFREE(char *) value = NULL;
>       unsigned int refcount = 0;
> -    int rc;
>   
>       *label = NULL;
>   
> -    if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
> -        return rc;
> -
>       if (!(ref_name = virSecurityGetRefCountAttrName(name)))
>           return -1;
>   
> @@ -288,6 +284,14 @@ virSecurityGetRememberedLabel(const char *name,
>                                ref_name,
>                                path);
>           return -1;
> +    } else {
> +        int rc;
> +
> +        if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
> +            return rc;
> +
> +        if (rc == 1)
> +            return -2;
>       }
>   
>       if (virStrToLong_ui(value, NULL, 10, &refcount) < 0) {
> @@ -357,10 +361,6 @@ virSecuritySetRememberedLabel(const char *name,
>       VIR_AUTOFREE(char *) attr_name = NULL;
>       VIR_AUTOFREE(char *) value = NULL;
>       unsigned int refcount = 0;
> -    int rc;
> -
> -    if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
> -        return rc;
>   
>       if (!(ref_name = virSecurityGetRefCountAttrName(name)))
>           return -1;
> @@ -375,6 +375,14 @@ virSecuritySetRememberedLabel(const char *name,
>                                    path);
>               return -1;
>           }
> +    } else {
> +        int rc;
> +

This needs to be executed if and only if @value is non-NULL otherwise 
the warning is going to be printed. Also, I'm adding a small comment 
here to explain why this is done AFTER @value is read.

> +        if ((rc = virSecurityValidateTimestamp(name, path)) < 0)
> +            return rc;
> +
> +        if (rc == 1)
> +            VIR_FREE(value);

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

and pushed.

Thanks,
Michal