[PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary

[Christof Schmitt]

On Fri, Apr 29, 2016 at 04:39:38PM -0700, Jeremy Allison wrote:
> On Fri, Apr 29, 2016 at 11:02:04AM -0700, Christof Schmitt wrote:
> > From 45a42c1a3fb2aad6efe3e8fd806adb7ba834fab0 Mon Sep 17 00:00:00 2001
> > From: Christof Schmitt <cs at samba.org>
> > Date: Tue, 26 Apr 2016 13:09:09 -0700
> > Subject: [PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary
> > 
> > Samba always tries to read the ACL of a file and checks it internally.
> > If the READ_ACL permission is missing in GPFS, then then reading the ACL
> > for Samba internal evaluation will be denied and opening the file or
> > directory fails. Change this by retrying reading the ACL with the DAC
> > capability if access was denied.
> 
> Comments inline below. Code with capabilities always scares me,
> but I think this is mostly right :-).
> 
> > Signed-off-by: Christof Schmitt <cs at samba.org>
> > ---
> >  source3/modules/vfs_gpfs.c | 30 ++++++++++++++++++++++++++++--
> >  1 file changed, 28 insertions(+), 2 deletions(-)
> > 
> > diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
> > index 42a3c72..ae2c598 100644
> > --- a/source3/modules/vfs_gpfs.c
> > +++ b/source3/modules/vfs_gpfs.c
> > @@ -358,6 +358,25 @@ static void gpfs_dumpacl(int level, struct gpfs_acl *gacl)
> >  	}
> >  }
> >  
> > +static int vfs_gpfs_getacl_cap(bool use_capability, const char *fname,
> > +			       int flags, void *aclbuf)
> > +{
> > +	int ret;
> > +
> > +	if (use_capability) {
> > +		set_effective_capability(DAC_OVERRIDE_CAPABILITY);
> > +	}
> > +
> > +	errno = 0;
> > +	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
> 
> I think you need to check for ret == -1 here and if so
> save off errno.
> 
> > +	if (use_capability) {
> > +		drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
> > +	}
> > +
> 
> Then restore it here in order for vfs_gpfs_getacl_cap()
> to correctly return a ret/errno pair.

Thank you for catching this. It looks like a successful call to
drop_effective_capability preserves the errno status, since it worked in
my testing, but the safer approach is to locally keep errno. See the
attached patch where this change is included.

Christof
-------------- next part --------------
From bfb3425ea310919d869bed69ac2bfd95ed02a23b Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 26 Apr 2016 13:09:09 -0700
Subject: [PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary

Samba always tries to read the ACL of a file and checks it internally.
If the READ_ACL permission is missing in GPFS, then then reading the ACL
for Samba internal evaluation will be denied and opening the file or
directory fails. Change this by retrying reading the ACL with the DAC
capability if access was denied.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/modules/vfs_gpfs.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index 42a3c72..3c03dd6 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -358,6 +358,27 @@ static void gpfs_dumpacl(int level, struct gpfs_acl *gacl)
 	}
 }
 
+static int vfs_gpfs_getacl_cap(bool use_capability, const char *fname,
+			       int flags, void *aclbuf)
+{
+	int ret, saved_errno;
+
+	if (use_capability) {
+		set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+	}
+
+	errno = 0;
+	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
+	saved_errno = errno;
+
+	if (use_capability) {
+		drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+	}
+
+	errno = saved_errno;
+	return ret;
+}
+
 /*
  * get the ACL from GPFS, allocated on the specified mem_ctx
  * internally retries when initial buffer was too small
@@ -378,6 +399,7 @@ static void *vfs_gpfs_getacl(TALLOC_CTX *mem_ctx,
 	int ret, flags;
 	unsigned int *len;
 	size_t struct_size;
+	bool use_capability = false;
 
 again:
 
@@ -406,8 +428,14 @@ again:
 	/* set the length of the buffer as input value */
 	*len = size;
 
-	errno = 0;
-	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
+	ret = vfs_gpfs_getacl_cap(use_capability, fname, flags, aclbuf);
+
+	if ((ret != 0) && (errno == EACCES) && !use_capability) {
+		DBG_DEBUG("Retry with DAC capability for %s\n", fname);
+		use_capability = true;
+		ret = vfs_gpfs_getacl_cap(use_capability, fname, flags, aclbuf);
+	}
+
 	if ((ret != 0) && (errno == ENOSPC)) {
 		/*
 		 * get the size needed to accommodate the complete buffer
-- 
1.8.3.1