[libvirt] [PATCH v4 3/3] security_dac: Favour ACLs over chown()

[Daniel P. Berrange]

On Fri, Mar 15, 2013 at 03:12:03PM +0100, Michal Privoznik wrote:
>  static int
> +virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid)
> +{
> +    int refCount = 0;
> +    bool xattrSupported = true;
> +
> +    VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'",
> +             path, (long) uid, (long) gid);
> +
> +    if (virSecurityDACGetXATTRRefcount(path, &refCount) < 0) {
> +        if (errno != ENOSYS && errno != ENOTSUP)
> +            return -1;

It is unsafe to check errno. You must use the virErrorPtr only,
and if you decide to ignore the error, you should also call
virResetLastError() to clear it.

> +        xattrSupported = false;
> +    }
> +
> +    if (refCount || virSecurityDACSetACL(path, uid) == 0) {
> +        if (xattrSupported &&
> +            virSecurityDACSetXATTRRefcount(path, refCount + 1) < 0) {
> +            /* Clear out oldACL XATTR */
> +            return -1;
> +        }
> +        return 0;
> +    }
> +
> +    /* Setting ACL failed. If the cause is libvirt was build without ACL
> +     * support, or filesystem does not support ACLs fall back to chown */
> +    if (errno != ENOSYS && errno != ENOTSUP)
> +        return -1;
> +
> +    VIR_DEBUG("Falling back to chown");
> +    if (xattrSupported && virSecurityDACRememberLabel(path) < 0)
> +        return -1;
> +
> +    if (virSecurityDACChown(path, uid, gid) < 0 ||
> +        (xattrSupported &&
> +         virSecurityDACSetXATTRRefcount(path, refCount + 1) < 0)) {
> +        /* XXX Clear our oldOwner XATTR */
> +        return -1;
> +    }
> +    return 0;
> +}

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|