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

On filesystems supporting ACLs we don't need to do a chown but we can just set ACLs to gain access for qemu. However, since we are setting these on too low level, where we don't know if disk is just a read only or read write, we set read write access unconditionally. >From implementation POV, a reference counter is introduced, so ACL is restored only on the last restore attempt in order to not cut off other domains. And since a file may had an ACL for a user already set, we need to keep this as well. Both these, the reference counter and original ACL are stored as extended attributes named trusted.refCount and trusted.oldACL respectively. --- diff to v2: -basically squashed functionality of 2/4 and 4/4 from previous round src/security/security_dac.c | 209 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 182 insertions(+), 27 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 0b274b7..46cc686 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -25,6 +25,7 @@ #include "security_dac.h" #include "virerror.h" +#include "virfile.h" #include "virutil.h" #include "viralloc.h" #include "virlog.h" @@ -34,6 +35,8 @@ #define VIR_FROM_THIS VIR_FROM_SECURITY #define SECURITY_DAC_NAME "dac" +#define SECURITY_DAC_XATTR_OLD_ACL "trusted.oldACL" +#define SECURITY_DAC_XATTR_REFCOUNT "trusted.refCount"

@@ -265,11 +406,8 @@ static const char * virSecurityDACGetDOI(virSecurityManagerPtr mgr ATTRIBUTE_UNU } static int -virSecurityDACSetOwnership(const char *path, uid_t uid, gid_t gid) +virSecurityDACChown(const char *path, uid_t uid, gid_t gid) { - VIR_INFO("Setting DAC user and group on '%s' to '%ld:%ld'", - path, (long) uid, (long) gid); - if (chown(path, uid, gid) < 0) { struct stat sb; int chown_errno = errno;

+ + /* Oops, something went wrong. If ACL or XATTR are not + * supported fall back to chown then. */ - /* XXX record previous ownership */ - rc = virSecurityDACSetOwnership(newpath, 0, 0); + rc = virSecurityDACChown(path, 0, 0);