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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxx>
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
